Bug 147249 - REGRESSION (r184026): Safari AutoFill with Contact info for phone number is broken
Summary: REGRESSION (r184026): Safari AutoFill with Contact info for phone number is b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: mitz
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2015-07-23 19:24 PDT by mitz
Modified: 2015-07-26 20:31 PDT (History)
4 users (show)

See Also:


Attachments
Use custom string coding for NSString and NSMutableString only (1.90 KB, patch)
2015-07-23 21:45 PDT, mitz
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2015-07-23 19:24:21 PDT
rdar://problem/21929532

Safari AutoFill is failing to fill some contact information.

The reason appears to be that an object graph being sent via WKRemoteObjectRegistry includes NSMutableString instances, and after <http://trac.webkit.org/changeset/184026> those aren’t being decoded correctly: that change applies custom encoding for NSString and its subclasses, but only the corresponding custom decoding for NSString itself.

Patch forthcoming.
Comment 1 mitz 2015-07-23 21:45:05 PDT
Created attachment 257435 [details]
Use custom string coding for NSString and NSMutableString only
Comment 2 mitz 2015-07-23 21:55:10 PDT
Fixed in <http://trac.webkit.org/r187288>.
Comment 3 Darin Adler 2015-07-26 20:16:36 PDT
Given that NSString is a class cluster, allocating a string with methods of NSString can create an object of a class derived from NSString, not NSString itself. Given that, I’m surprised this code works, and I think we may need a different approach to remain robust in the future.
Comment 4 mitz 2015-07-26 20:27:45 PDT
(In reply to comment #3)
> Given that NSString is a class cluster, allocating a string with methods of
> NSString can create an object of a class derived from NSString, not NSString
> itself. Given that, I’m surprised this code works, and I think we may need a
> different approach to remain robust in the future.

Note that the objectClass variable in encodeObject() is initialized with [object classForCoder], not [object class]. The NSString implementation of -classForCoder returns NSString, and the concrete subclasses (in Foundation) don’t override it, so they all encode as NSString. Similarly, all (Foundation) concrete subclasses of NSMutableString encode as NSMutableString.

The assumption here is that anything that encodes as an NSString or NSMutableString uses the Foundation encoder that has the bug that r184026 was meant to work around, and that any subclass that has a different classForCoder probably uses a different encoder that doesn’t have the bug.
Comment 5 Darin Adler 2015-07-26 20:31:00 PDT
OK, great!