Bug 159555

Summary: adoptNode() changes css class to lowercase for document loaded with XHR responseType = "document"
Product: WebKit Reporter: MarkG <theblueslate>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, commit-queue, esprehn+autocc, kangil.han, kling, koivisto, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 9   
Hardware: Mac   
OS: OS X 10.11   
URL: https://jsfiddle.net/theblueslate/wxo7zst5/2/
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews114 for mac-yosemite
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description MarkG 2016-07-08 05:17:46 PDT
For a full demonstration, see https://jsfiddle.net/theblueslate/wxo7zst5/2/

I use XHR with responseType = "document" to load html fragments dynamically. To include them in the document, I use adoptNode(). However, if the XHR parsed document is from a document in quirks mode, and the contents contains a css class that includes an uppercase letter, then adoptNode() imports the element with the css class in lowercase.

importNode() doesn't have this problem, but is more heavyweight than adoptNode().

I can reproduce this problem on Safari 9.1.1 on El Capitan. I also have the same problem on iOS 9.3.2. This problem does not occur on Chrome 51 or IE11.
Comment 1 Chris Dumez 2016-07-08 13:40:01 PDT
I have a fix that works for this test case but I need to do more validation.
Comment 2 Radar WebKit Bug Importer 2016-07-08 13:40:19 PDT
<rdar://problem/27252541>
Comment 3 Chris Dumez 2016-07-08 15:28:51 PDT
Created attachment 283203 [details]
Patch
Comment 4 Chris Dumez 2016-07-08 16:07:03 PDT
Comment on attachment 283203 [details]
Patch

Breaks a W3C test.
Comment 5 Build Bot 2016-07-08 16:22:36 PDT
Comment on attachment 283203 [details]
Patch

Attachment 283203 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1649107

New failing tests:
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html
Comment 6 Build Bot 2016-07-08 16:22:40 PDT
Created attachment 283210 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-07-08 16:23:22 PDT
Comment on attachment 283203 [details]
Patch

Attachment 283203 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1649136

New failing tests:
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html
Comment 8 Build Bot 2016-07-08 16:23:27 PDT
Created attachment 283211 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-07-08 16:26:28 PDT
Comment on attachment 283203 [details]
Patch

Attachment 283203 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1649137

New failing tests:
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html
Comment 10 Build Bot 2016-07-08 16:26:33 PDT
Created attachment 283212 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 11 Chris Dumez 2016-07-08 16:28:07 PDT
Created attachment 283214 [details]
Patch
Comment 12 Chris Dumez 2016-07-08 16:44:52 PDT
Created attachment 283222 [details]
Patch
Comment 13 Ryosuke Niwa 2016-07-08 17:08:11 PDT
Comment on attachment 283222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283222&action=review

> LayoutTests/fast/dom/Document/adoptNode-quirks-mismatch.html:50
> +var xhr = new XMLHttpRequest();

Nit: indent this line.

> LayoutTests/fast/dom/Document/adoptNode-quirks-mismatch.html:53
> +    xhr.open("GET", tests[i], true);

Why don't we just do a sync XHR instead?  That'll make this test much easier to run and make it less racy.
This test won't be flaky because shouldBeEqualToString doesn't contain any document specific string
but we can't tell which case failed by just looking at the result because either document can be loaded first if async is set true.

Alternatively, you can trigger each XHR after the previous test had finished.
i.e. execute the next test in the else clause of documentLoaded.
Comment 14 Chris Dumez 2016-07-08 18:23:28 PDT
Created attachment 283234 [details]
Patch
Comment 15 MarkG 2016-07-08 18:27:35 PDT
@Ryosuke I believe XHR.responseType = "document" only works in async. See: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType
Comment 16 Chris Dumez 2016-07-08 18:36:46 PDT
Created attachment 283236 [details]
Patch
Comment 17 WebKit Commit Bot 2016-07-08 19:25:11 PDT
Comment on attachment 283236 [details]
Patch

Clearing flags on attachment: 283236

Committed r203018: <http://trac.webkit.org/changeset/203018>
Comment 18 WebKit Commit Bot 2016-07-08 19:25:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Chris Dumez 2016-07-09 08:10:11 PDT
The fix is incomplete.
Comment 20 Chris Dumez 2016-07-09 08:14:43 PDT
Created attachment 283260 [details]
Patch
Comment 21 WebKit Commit Bot 2016-07-10 16:28:49 PDT
Comment on attachment 283260 [details]
Patch

Clearing flags on attachment: 283260

Committed r203043: <http://trac.webkit.org/changeset/203043>
Comment 22 WebKit Commit Bot 2016-07-10 16:28:55 PDT
All reviewed patches have been landed.  Closing bug.