| Summary: | Changes to slot children should trigger slotchange | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jan Miksovsky <jan> | ||||||||||||||||||
| Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | cdumez, cmarcelo, darin, dbates, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, rniwa, webkit-bug-importer | ||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
| Version: | Safari Technology Preview | ||||||||||||||||||||
| Hardware: | Mac | ||||||||||||||||||||
| OS: | macOS 10.12 | ||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||
| Bug Blocks: | 148695 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Jan Miksovsky
2017-03-15 16:31:32 PDT
Just spent an hour isolating a repro for a WebKit bug — and discovered that it's the same as this bug I'd filed half a year ago. FWIW, here's a more complex repro involving an outer element and an inner element: http://jsbin.com/raciraw/edit?html,output. the outer element changes its default slot content, which changes what's assigned to the inner element's slot — but the inner element's slot never raises slotchange. This works as expected in Blink, but not in WebKit. Created attachment 348058 [details]
WIP
*** Bug 188752 has been marked as a duplicate of this bug. *** Created attachment 348262 [details]
WIP2
There is quite a bit of change involved in order to not keep resolving the slot assignments. A very native implementation would end up resolving the assignments of each slot whenever a child node is inserted or removed from a slot. I don't think that's acceptable implementation. Ugh... this is really shitty. There is no way to avoid at least one O(n) operation when the first slot is inserted because we'd have to figure out whether each slot uses its assigned content or fallback because we don't want to do be doing all the house-keeping when there are no slows in a shadow tree. Comment on attachment 348262 [details] WIP2 Attachment 348262 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9006368 New failing tests: imported/w3c/web-platform-tests/shadow-dom/slotchange.html Created attachment 348266 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 348262 [details] WIP2 Attachment 348262 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9007251 New failing tests: imported/w3c/web-platform-tests/shadow-dom/slotchange.html Created attachment 348276 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Created attachment 348364 [details]
Fixes the bug
Comment on attachment 348364 [details] Fixes the bug Attachment 348364 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/9017745 New failing tests: imported/w3c/web-platform-tests/shadow-dom/slotchange.html Created attachment 348374 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Created attachment 348382 [details]
Fixed rebaseline
Comment on attachment 348382 [details]
Fixed rebaseline
Wait, why no test progression in the patch? I see a change log entry, but no test result.
(In reply to Darin Adler from comment #16) > Comment on attachment 348382 [details] > Fixed rebaseline > > Wait, why no test progression in the patch? I see a change log entry, but no > test result. Yeah, there was a bug in this latest patch. I'm uploading a new patch with more test cases. The test in WPT isn't so good. Created attachment 348390 [details]
Fixes the bug
Thanks for the review! Committed r235458: <https://trac.webkit.org/changeset/235458> |