Bug 104275 - Leaf nodes should use parent tree scope
Summary: Leaf nodes should use parent tree scope
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-06 10:38 PST by Elliott Sprehn
Modified: 2013-01-04 14:35 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.77 KB, patch)
2012-12-06 10:53 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff
Patch (2.80 KB, patch)
2012-12-06 12:54 PST, Elliott Sprehn
morrita: review+
morrita: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2012-12-06 10:38:56 PST
Text nodes should use parent tree scope
Comment 1 Elliott Sprehn 2012-12-06 10:53:52 PST
Created attachment 178037 [details]
Patch
Comment 2 WebKit Review Bot 2012-12-06 12:06:23 PST
Comment on attachment 178037 [details]
Patch

Attachment 178037 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15159784

New failing tests:
editing/selection/select-across-readonly-input-5.html
editing/pasteboard/copy-null-characters.html
editing/input/paste-text-ending-with-interchange-newline.html
dom/xhtml/level3/core/nodeinsertbefore02.xhtml
editing/pasteboard/drop-text-events.html
editing/pasteboard/drag-drop-input-textarea.html
http/tests/local/file-url-sent-as-referer.html
editing/pasteboard/5665299.html
editing/selection/select-across-readonly-input-4.html
dom/xhtml/level3/core/documentadoptnode12.xhtml
editing/pasteboard/drag-drop-url-text.html
fast/canvas/canvas-bg-multiple-removal.html
dom/xhtml/level3/core/documentgetxmlstandalone05.xhtml
editing/selection/select-across-readonly-input-1.html
editing/selection/crash-on-drag-with-mutation-events.html
editing/shadow/shadow-selection-not-exported.html
editing/shadow/compare-positions-in-nested-shadow.html
editing/pasteboard/drop-inputtext-acquires-style.html
editing/pasteboard/copy-paste-pre-line-content.html
editing/pasteboard/copy-paste-first-line-in-textarea.html
editing/shadow/selection-of-shadowroot.html
editing/selection/select-across-readonly-input-3.html
fast/css-generated-content/block-after.html
editing/spelling/spellcheck-async-mutation.html
dom/xhtml/level3/core/nodereplacechild13.xhtml
fast/dom/XMLSerializer-doctype.html
fast/dom/createDocumentType2.html
editing/selection/select-across-readonly-input-2.html
dom/xhtml/level3/core/nodecomparedocumentposition02.xhtml
dom/xhtml/level3/core/documentgetdoctype01.xhtml
WebFrameTest.FindInPage
Comment 3 Dimitri Glazkov (Google) 2012-12-06 12:14:17 PST
You've made Ews elves angry.
Comment 4 Elliott Sprehn 2012-12-06 12:45:06 PST
(In reply to comment #2)
> (From update of attachment 178037 [details])
> Attachment 178037 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15159784
> 
> New failing tests:
> ..

Woah, that seems crazy I could have failed so many tests... looking into this.
Comment 5 Elliott Sprehn 2012-12-06 12:49:44 PST
Comment on attachment 178037 [details]
Patch

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

> Source/WebCore/dom/Document.h:1555
> +    if (isContainerNode() && parentNode())

Err I'm dumb. This should be !isContainerNode(). I originally had isTextNode() and changed it.
Comment 6 Elliott Sprehn 2012-12-06 12:54:20 PST
Created attachment 178065 [details]
Patch
Comment 7 Hajime Morrita 2012-12-06 21:49:47 PST
Comment on attachment 178065 [details]
Patch

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

This is clever. cq- since this seems incompatible to your another patch.

> Source/WebCore/dom/Document.h:1557
> +    if (!isContainerNode() && parentNode())

This can be tuned a bit more
- parentNode() has one flag check so the result better be stored.
- if !isContainerNode(), it cannot be in shadow, so the tree scope is always document.
Comment 8 Elliott Sprehn 2013-01-04 14:35:25 PST
My patch on Bug 59816 seems to have worked as I'm not seeing any perf regressions on the page cyclers so we don't need this anymore.