Bug 18976 – REGRESSION (r26474): WebKit fails jQuery test 64 core module: text(String) subtest 1 Check escaped text (createTextNode)

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[David Kilzer (:ddkilzer)]

* SUMMARY
When running the jQuery test suite, WebKit (in the form of Safari 3.1.1 on Leopard 10.5.x), fails one of the tests:

64. core module: text(String) (1, 3, 4)
    1. Check escaped text

This is either a real failure or an evangelism issue.

* STEPS TO REPRODUCE
1. Launch Safari/WebKit.
2. Open URL:  http://jquery.com/test/
3. Wait for test to complete.

* RESULTS
WebKit fails 1 of 1157 tests (see SUMMARY for details).

* REGRESSION
Unknown.  Only tested with Safari 3.1.1.

[David Kilzer (:ddkilzer)]

<rdar://problem/5924793>

[David Kilzer (:ddkilzer)]

The issue is how document.createTextNode() encodes text.

[David Kilzer (:ddkilzer)]

Created attachment 21052 [details]
Test case

[David Kilzer (:ddkilzer)]

Comment on attachment 21052 [details]
Test case

This test case is bad.

When the Expected and Actual lines match, the test still fails.

[David Kilzer (:ddkilzer)]

(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.

[David Kilzer (:ddkilzer)]

The bisect-builds script with Safari 2.0.4 reports:

Works: r26359  Fails: r26570

[David Kilzer (:ddkilzer)]

Regression occurred in r26474 (noted in comments):

http://trac.webkit.org/projects/webkit/changeset/26474

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

Created attachment 21127 [details]
Patch v1

Proposed fix.

[Darin Adler]

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

[Tor Arne Vestbø]

Thanks for the heads up David.

[David Kilzer (:ddkilzer)]

Committed revision 33451.  Removed unnecessary optimization per Comment #10.

Tor/Simon:  Please update Qt test results per Comment #8.  Thanks!

[Alexey Proskuryakov]

*** Bug 18101 has been marked as a duplicate of this bug. ***