RESOLVED FIXED 24755
LayoutTests/http/tests/misc/url-in-utf16le.html regression
https://bugs.webkit.org/show_bug.cgi?id=24755
Summary LayoutTests/http/tests/misc/url-in-utf16le.html regression
Nico Weber
Reported 2009-03-22 22:00:12 PDT
WebKit change 41727 regressed query param handling for utf16 files. These fail: WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16le.html WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16be.html
Attachments
Patch. (1.25 KB, patch)
2009-03-22 22:04 PDT, Nico Weber
no flags
Patch with better ChangeLog. (1.32 KB, patch)
2009-03-22 22:07 PDT, Nico Weber
no flags
Patch with better changelog and no tabs. (1.32 KB, patch)
2009-03-22 22:09 PDT, Nico Weber
darin: review-
patch that fixes regression tests without reintroducing the performance problem (4.58 KB, patch)
2009-03-23 09:17 PDT, Darin Adler
koivisto: review+
Nico Weber
Comment 1 2009-03-22 22:04:48 PDT
Created attachment 28845 [details] Patch. Revert change 41727. Also fixes http://crbug.com/9067 .
Nico Weber
Comment 2 2009-03-22 22:05:33 PDT
With this patch, WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16le.html works fine again.
Nico Weber
Comment 3 2009-03-22 22:07:48 PDT
Created attachment 28846 [details] Patch with better ChangeLog.
Nico Weber
Comment 4 2009-03-22 22:09:44 PDT
Created attachment 28847 [details] Patch with better changelog and no tabs. Sorry for the noise. It's my first time.
Nico Weber
Comment 5 2009-03-22 22:16:16 PDT
Some comments on the change: If `this` is an UTF16-encoding, change 41727 caused wrong results. Since `isNonByteBasedEncoding()` checks `noExtendedTextEncodingNameUsed()` already, and boolean operators are lazy, reverting 41727 does not cause any harm but fixes the regression.
David Kilzer (:ddkilzer)
Comment 6 2009-03-23 09:08:39 PDT
Darin Adler
Comment 7 2009-03-23 09:08:48 PDT
Comment on attachment 28847 [details] Patch with better changelog and no tabs. We can't just roll out this optimization, which was fixing a significant performance problem. We need a new fix rather than just rolling out the bad one. Let me take a look at this.
Darin Adler
Comment 8 2009-03-23 09:10:34 PDT
Comment on attachment 28847 [details] Patch with better changelog and no tabs. I'm about to post an improved patch for this.
Darin Adler
Comment 9 2009-03-23 09:17:12 PDT
Created attachment 28854 [details] patch that fixes regression tests without reintroducing the performance problem
Antti Koivisto
Comment 10 2009-03-23 11:12:55 PDT
r=me
Darin Adler
Comment 11 2009-03-23 16:50:28 PDT
Note You need to log in before you can comment on or make changes to this bug.