Bug 121609 - Remove URL decoding in srcset handling
Summary: Remove URL decoding in srcset handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 121695
  Show dependency tree / graph
 
Reported: 2013-09-19 07:47 PDT by Yoav Weiss
Modified: 2013-09-20 10:48 PDT (History)
9 users (show)

See Also:

Attachments
Patch (7.06 KB, patch)
2013-09-19 07:53 PDT, Yoav Weiss 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yoav Weiss 2013-09-19 07:47:42 PDT 
Remove URL decoding in srcset handling
Comment 1 Yoav Weiss 2013-09-19 07:53:42 PDT 
Created attachment 212068 [details]
Patch
Comment 2 Yoav Weiss 2013-09-19 07:59:26 PDT 
As pointed out to me by Blink's Christian Biesinger, the URL decoding in the srcset algorithm can break encoded URLs (both in src and in srcset)

Since the new parser is in place (which splits the attribute on white spaces rather than commas), data URIs are handled properly, and decoding URLs doesn't really offer any benefits. Therefore, I removed decoding to avoid breaking these URLs.
Comment 3 Benjamin Poulain 2013-09-19 14:48:38 PDT 
Thanks for the follow up.
Comment 4 WebKit Commit Bot 2013-09-19 22:01:55 PDT 
Comment on attachment 212068 [details]
Patch

Clearing flags on attachment: 212068

Committed r156140: <http://trac.webkit.org/changeset/156140>
Comment 5 WebKit Commit Bot 2013-09-19 22:01:58 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Patrick R. Gansterer 2013-09-20 06:11:38 PDT 
(In reply to comment #4)
> (From update of attachment 212068 [details])
> Clearing flags on attachment: 212068
> 
> Committed r156140: <http://trac.webkit.org/changeset/156140>

This change broke development on native windows systems. '?' is not a valid character in filenames on windows.
Can you fix the filename (by changing the test to a sever side script which checks for the question mark or any other special character in the URL) or roll out the patch?
Comment 7 Yoav Weiss 2013-09-20 06:18:41 PDT 
(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 212068 [details] [details])
> > Clearing flags on attachment: 212068
> > 
> > Committed r156140: <http://trac.webkit.org/changeset/156140>
> 
> This change broke development on native windows systems. '?' is not a valid character in filenames on windows.
> Can you fix the filename (by changing the test to a sever side script which checks for the question mark or any other special character in the URL) or roll out the patch?

I'm extremely sorry :(
Will fix the filename ASAP
Comment 8 Alexey Proskuryakov 2013-09-20 10:02:54 PDT 
Looks like Windows was fixed in <http://trac.webkit.org/changeset/156161>.
Comment 9 Alexey Proskuryakov 2013-09-20 10:27:41 PDT 
And then more in <http://trac.webkit.org/r156179>.
Comment 10 Alexey Proskuryakov 2013-09-20 10:37:59 PDT 
It should be possible to test question marks with a cgi script that intercepts all requests within a directory. We do this in LayoutTests/http/tests/uri/intercept tests.