Bug 167979 - Implement URL's toJSON()
Summary: Implement URL's toJSON()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-08 01:27 PST by Anne van Kesteren
Modified: 2017-02-11 07:16 PST (History)
4 users (show)

See Also:


Attachments
WIP Patch (10.96 KB, patch)
2017-02-10 15:38 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.89 KB, patch)
2017-02-10 20:39 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Chris Dumez 2017-02-10 15:38:46 PST
Created attachment 301204 [details]
WIP Patch
Comment 2 Alex Christensen 2017-02-10 15:47:15 PST
I think we should add a test like "https://example.com/#\"" to verify what happens when a " appears in the serialized URL.
Comment 3 Chris Dumez 2017-02-10 16:02:53 PST
(In reply to comment #2)
> I think we should add a test like "https://example.com/#\"" to verify what
> happens when a " appears in the serialized URL.

In Firefox nightly:
JSON.stringify(new URL("https://example.com/#\""))  
 "https://example.com/#%22"

With my patch:
JSON.stringify(new URL("https://example.com/#\""))
 "\"https://example.com/#\\\"\""
Comment 4 Chris Dumez 2017-02-10 19:22:18 PST
(In reply to comment #3)
> (In reply to comment #2)
> > I think we should add a test like "https://example.com/#\"" to verify what
> > happens when a " appears in the serialized URL.
> 
> In Firefox nightly:
> JSON.stringify(new URL("https://example.com/#\""))  
>  "https://example.com/#%22"
> 
> With my patch:
> JSON.stringify(new URL("https://example.com/#\""))
>  "\"https://example.com/#\\\"\""

Alex, any thoughts on this? Not super familiar with this so I am not sure what's expected. The URL serialization merely says to append the fragment.

Either way, this would be an issue with our serialization and would also impact the stringier, not only the new toJSON.
Comment 5 Chris Dumez 2017-02-10 20:39:15 PST
Created attachment 301240 [details]
Patch
Comment 6 Anne van Kesteren 2017-02-10 23:44:52 PST
Chris, the difference you are seeing is because Firefox encodes more code points during parsing of the URL fragment. Firefox is the only browser to do so, so I think the results of your patch are indeed what you should be seeing here. See https://url.spec.whatwg.org/#fragment-state in particular.
Comment 7 Chris Dumez 2017-02-11 06:50:10 PST
(In reply to comment #6)
> Chris, the difference you are seeing is because Firefox encodes more code
> points during parsing of the URL fragment. Firefox is the only browser to do
> so, so I think the results of your patch are indeed what you should be
> seeing here. See https://url.spec.whatwg.org/#fragment-state in particular.

Thanks for confirming.
Comment 8 WebKit Commit Bot 2017-02-11 07:16:23 PST
Comment on attachment 301240 [details]
Patch

Clearing flags on attachment: 301240

Committed r212193: <http://trac.webkit.org/changeset/212193>
Comment 9 WebKit Commit Bot 2017-02-11 07:16:29 PST
All reviewed patches have been landed.  Closing bug.