Bug 53717

<rdar://problem/8955448>

Minefield doesn't have this bug.

I see this error in the Web Inspector: TypeError: 'undefined' is not a function (evaluating '$('#skybox_slideshow').cycle({timeout:4000,pause:true,speed:800})')

Nothing obvious jumps out at my from looking at the page. We'll need to create a reduction.

Created attachment 81120 [details] Test case Reduction attached. The issue seems to be with this snippet: <script src=" http://media3.washingtonpost.com/wp-srv/wpost/javascript/plugin/plugin.jquery.cycle.all-2.88.0.js " type="text/javascript" charset="utf-8"></script> If you remove the line break between 'src="' and 'http://...' the bug goes away.

Just to explain the test case, the correct behavior is to see text that cycles between the words 'FIRST' and 'SECOND' once every 800 ms. If you see both 'FIRST' and 'SECOND' at the same time, this is a bug.

Interesting. This is probably related to the uncertainty as whose job it is to trim whitespace from before and after URLs. Should be easy to fix. The question is mostly at which layer to trim the whitespace.

This is a bug in KURL. The test case works fine with GURL. KURL is supposed to trim leading whitespace from URLs.

Will fix.

There were already tests for this in fast/url/scheme.html, but they're disabled: // These tests trigger the relative URL resolving behavior of // HTMLAnchorElement.href. In order to test absolute URL parsing, we'd need // an API that always maps to absolute URLs. If you know of one, please // enable these tests! // [" HTTP ", "%20http%20"], // ["htt: ", "htt%3A%20"], // ["\xe4\xbd\xa0\xe5\xa5\xbdhttp", "%E4%BD%A0%E5%A5%BDhttp"], // ["ht%3Atp", "ht%3atp"], I'll add the parser version of this test case since that's testable (as demonstrated by the attachment).

Created attachment 82566 [details] Patch

Comment on attachment 82566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82566&action=review > Source/WebCore/platform/KURL.cpp:349 > + // characters from URLs. Note that c is an *unsigned* char here > + // so this comparison should only catch control characters. Is this really worth explaining in a comment?

Comment on attachment 82566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82566&action=review I believe this change is correct. I don't really have an opinion on the comment. >> Source/WebCore/platform/KURL.cpp:349 >> + // Browsers ignore leading/trailing whitespace and control >> + // characters from URLs. Note that c is an *unsigned* char here >> + // so this comparison should only catch control characters. > > Is this really worth explaining in a comment? Most of the rest of KURL uses plain "char", which can be subtly confusing.

(In reply to comment #12) > (From update of attachment 82566 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82566&action=review > > > Source/WebCore/platform/KURL.cpp:349 > > + // characters from URLs. Note that c is an *unsigned* char here > > + // so this comparison should only catch control characters. > > Is this really worth explaining in a comment? Most of the rest of KURL actually gets it wrong. :) Compares with < to a char and assumes it works. I added the comment after I got it wrong myself. I'll go through and fix the other KURL char compares in a separate patch.

Comment on attachment 82566 [details] Patch Clearing flags on attachment: 82566 Committed r78747: <http://trac.webkit.org/changeset/78747>

All reviewed patches have been landed. Closing bug.