Summary: | (register|unregister)DynamicSubtreeNodeList should be called only for labels and regions node lists | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||
Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | adamk, ap, arv, darin, dglazkov, kling, koivisto, mihnea, sam, tkent, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 81021 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2012-03-12 15:32:25 PDT
Created attachment 131427 [details]
cleanup
Comment on attachment 131427 [details]
cleanup
r=me.
Comment on attachment 131427 [details] cleanup Attachment 131427 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11941228 New failing tests: fast/forms/label/labels-remove-parent-label.html fast/forms/label/labels-set-htmlFor-attribute.html fast/forms/label/labels-remove-htmlFor-attribute.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/forms/label/labels-add-htmlFor-label.html fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/forms/label/labels-change-htmlFor-attribute.html fast/forms/label/labels-add-parent-label.html fast/forms/label/labels-remove-htmlFor-label.html fast/regions/get-regions-by-content-node2.html Comment on attachment 131427 [details]
cleanup
Ugh... this isn't quite right because of LabelsNodeList and RegionNodeList. They need to be notified of changes at the document level instead of at the node it belongs :(
Changing the scope of the bug a little bit. Created attachment 131501 [details]
Patch
Comment on attachment 131501 [details] Patch Attachment 131501 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11942360 New failing tests: fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/regions/get-regions-by-content-node.html fast/regions/get-regions-by-content-node2.html Created attachment 131560 [details] Also fix the bug 80638 Comment on attachment 131560 [details] Also fix the bug 80638 Attachment 131560 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11942407 Comment on attachment 131560 [details] Also fix the bug 80638 Attachment 131560 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11945371 New failing tests: fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/regions/get-regions-by-content-node.html fast/regions/get-regions-by-content-node2.html ugh.... regions node lists were passing tests due to "redundant" cache invalidation magically updating otherwise stale node lists :-( Created attachment 131777 [details]
Updated for ToT
Comment on attachment 131777 [details] Updated for ToT Attachment 131777 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11948868 Committed in http://trac.webkit.org/changeset/110797. Apparently I mangled patches together when I was landing a patch for https://bugs.webkit.org/show_bug.cgi?id=80706. Comment on attachment 131777 [details] Updated for ToT View in context: https://bugs.webkit.org/attachment.cgi?id=131777&action=review r=me but please remove references to region node lists from the code/changelog. Bonus points for removing a parseAttribute() overload. :) > Source/WebCore/dom/DynamicNodeList.h:90 > + DynamicNodeList::invalidateCache; Needs "using" to build on win, but you know that. The patch was "preemptively" landed in http://trac.webkit.org/changeset/110797 by accident. Fixed the reviewer name in http://trac.webkit.org/changeset/110817. |