Summary: | REGRESSION (r26474): WebKit fails jQuery test 64 core module: text(String) subtest 1 Check escaped text (createTextNode) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bill, hausmann, justin.garcia, vestbo | ||||||
Priority: | P2 | Keywords: | InRadar, NeedsReduction, Regression | ||||||
Version: | 525.x (Safari 3.1) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
URL: | http://jquery.com/test/ | ||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2008-05-09 14:15:48 PDT
The issue is how document.createTextNode() encodes text. Created attachment 21052 [details]
Test case
Comment on attachment 21052 [details]
Test case
This test case is bad.
When the Expected and Actual lines match, the test still fails.
(In reply to comment #0) > * REGRESSION > Unknown. Only tested with Safari 3.1.1. This is a regression as it works with Safari 3.0.4 (523.12.2) on Mac OS X 10.4.11 (8S165). I also have a fix. Regression occurred in r26474 (noted in comments): http://trac.webkit.org/projects/webkit/changeset/26474 Qt developers: The following test results will likely need to be updated after this bug is fixed: LayoutTests/platform/qt/fast/dom/dom-parse-serialize-expected.txt LayoutTests/platform/qt/fast/xsl/xslt-text-expected.txt Note that LayoutTests/fast/xsl/xslt-text-expected.txt is a text dump, while LayoutTests/platform/qt/fast/xsl/xslt-text-expected.txt is a render tree dump. You may be able to use the common test results and remove the Qt-specific results. Created attachment 21127 [details]
Patch v1
Proposed fix.
Comment on attachment 21127 [details]
Patch v1
Putting these strings into separate functions might make the code slightly slower unnecessarily. There's no particular reason these need to be shared globals, and I had originally used a C-style string because I thought that would be fast enough; widening the characters from 8-bit to 16-bit should be fast enough. And using globals at all (not new to this patch) creates unnecessary threading issues if we ever start tackling them the way we are in JavaScriptCore.
But aside from that entirely-theoretical worry, looks perfect.
r=me
Thanks for the heads up David. Committed revision 33451. Removed unnecessary optimization per Comment #10. Tor/Simon: Please update Qt test results per Comment #8. Thanks! *** Bug 18101 has been marked as a duplicate of this bug. *** |