Skip to content
Permalink
Browse files
Start removing functions that implicitly use composed tree
https://bugs.webkit.org/show_bug.cgi?id=218424

Reviewed by Ryosuke Niwa.

Source/WebCore:

Tests: fast/dom/rangeContainsNode.html
       fast/dom/rangeContainsRange.html
       fast/dom/treeOrder.html

Ryosuke pointed out that most clients should not be using the composed tree, so this
is a step in work to make the use of the composed tree explicit. After that we can
more easily find and correct call sites where use of the composed tree is incorrect.

Likely that most or all cases of treeOrder<ComposedTree> can be changed to just
treeOrder and contains<ComposedTree> to just contains; will do those in follow-ups.

* Modules/highlight/Highlight.cpp:
(WebCore::repaintRange): Simplified code to use boundary points, and changed to use
treeOrder<ComposedTree>.

* dom/AbstractRange.h: Export makeSimpleRange so it can be used in Internals.
* dom/AbstractRange.idl: Export the class so it can be used in Internals.

* dom/Node.cpp:
(WebCore::parent<ShadowIncludingTree>): Added.
(WebCore::treeOrderForTesting): Added.
(WebCore::documentOrder): Deleted.
* dom/Node.h: Updated for the above.

* dom/RadioButtonGroups.cpp:
(WebCore::RadioButtonGroup::members const): Use treeOrder<ComposedTree>.

* dom/SimpleRange.cpp:
(WebCore::contains): Removed a couple contains functions that implicitly use
composed tree.
(WebCore::containsForTesting): Added.
* dom/SimpleRange.h: Updated for the above. Should also make contains<Tree> the
default and remove the <Tree> at all call sites, but it's safer to do that in a
separate patch after this is landed and builds on all platforms.

* editing/Editing.cpp:
(WebCore::isNodeVisiblyContainedWithin): Use contains<ComposedTree>.
* page/DragController.cpp:
(WebCore::DragController::draggableElement const): Ditto.
* page/EventHandler.cpp:
(WebCore::EventHandler::dispatchMouseEvent): Ditto. Note that this has a call to
isDescendantOf right next to it, which does not use the composed tree.

* page/Page.cpp:
(WebCore::Page::findTextMatches): Use treeOrder<ComposedTree>.
(WebCore::replaceRanges): Ditto.

* page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::findTelephoneNumberHighlightContainingSelectionHighlight):
Use contains<ComposedTree>.

* testing/Internals.cpp:
(WebCore::string): Added.
(WebCore::convertType): Added.
(WebCore::Internals::treeOrder): Added.
(WebCore::Internals::rangeContainsNode): Added.
(WebCore::Internals::rangeContainsRange): Added.
* testing/Internals.h: Updated for added functions above.
* testing/Internals.idl: Ditto. These functions were tested in a
TestWebKitAPI test, but we plan to move those types of tests to
internals-based tests instead.

Source/WebKit:

* WebProcess/WebPage/glib/WebPageGLib.cpp:
(WebKit::WebPage::getPlatformEditorState const): Use contains<ComposedTree>.

Tools:

* TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp: Moved the tests of documentOrder
for two nodes and contains for range/range or range/node into internals-based tests
in the LayoutTests directory. Eventually we'll move the rest of these tests, too.

LayoutTests:

* fast/dom/rangeContainsNode-expected.txt: Added.
* fast/dom/rangeContainsNode.html: Added.
* fast/dom/rangeContainsRange-expected.txt: Added.
* fast/dom/rangeContainsRange.html: Added.
* fast/dom/treeOrder-expected.txt: Added.
* fast/dom/treeOrder.html: Added.
These tests were formerly part of TestWebKitAPI, although treeOrder was named documentOrder.
Moving to internals-style tests at Ryosuke's suggestion since these are not something exposed
as API or even SPI.

Canonical link: https://commits.webkit.org/231111@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@269253 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
darinadler committed Nov 2, 2020
1 parent 9624053 commit 185f4954b1936bb142084131082de0b1b6e05432
Showing 28 changed files with 580 additions and 215 deletions.
@@ -1,3 +1,20 @@
2020-11-01 Darin Adler <darin@apple.com>

Start removing functions that implicitly use composed tree
https://bugs.webkit.org/show_bug.cgi?id=218424

Reviewed by Ryosuke Niwa.

* fast/dom/rangeContainsNode-expected.txt: Added.
* fast/dom/rangeContainsNode.html: Added.
* fast/dom/rangeContainsRange-expected.txt: Added.
* fast/dom/rangeContainsRange.html: Added.
* fast/dom/treeOrder-expected.txt: Added.
* fast/dom/treeOrder.html: Added.
These tests were formerly part of TestWebKitAPI, although treeOrder was named documentOrder.
Moving to internals-style tests at Ryosuke's suggestion since these are not something exposed
as API or even SPI.

2020-11-02 Martin Robinson <mrobinson@igalia.com>

[GLIB] REGRESSION(r269144) imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-scrollMargin.html is failing
@@ -0,0 +1,22 @@
Test the contains function that checks if a range contains a node.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS internals.rangeContainsNode(contentsRange(document), documentElement, 'ComposedTree') is true
PASS internals.rangeContainsNode(contentsRange(document), body, 'ComposedTree') is true
PASS internals.rangeContainsNode(contentsRange(documentElement), body, 'ComposedTree') is true
PASS internals.rangeContainsNode(contentsRange(document), document, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, document, 2), document, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, documentElement, 0), body, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, documentElement, 1), body, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, body, 0), body, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, body, 1), body, 'ComposedTree') is false
PASS internals.rangeContainsNode(range(document, 0, documentElement, 2), body, 'ComposedTree') is true
PASS internals.rangeContainsNode(contentsRange(document), a, 'ComposedTree') is false
PASS internals.rangeContainsNode(contentsRange(body), f, 'ComposedTree') is false
PASS internals.rangeContainsNode(contentsRange(body), h, 'ComposedTree') is true
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,57 @@
<html><head></head><body>
<script src="../../resources/js-test.js"></script>
<script>

description("Test the contains function that checks if a range contains a node.")

function range(startContainer, startOffset, endContainer, endOffset)
{
return new StaticRange({ startContainer: startContainer, startOffset: startOffset, endContainer: endContainer, endOffset: endOffset })
}

function contentsRange(node)
{
return range(node, 0, node, node.childNodes.length)
}

let documentElement = document.documentElement;
let body = document.body;

shouldBeTrue("internals.rangeContainsNode(contentsRange(document), documentElement, 'ComposedTree')")
shouldBeTrue("internals.rangeContainsNode(contentsRange(document), body, 'ComposedTree')");
shouldBeTrue("internals.rangeContainsNode(contentsRange(documentElement), body, 'ComposedTree')");

shouldBeFalse("internals.rangeContainsNode(contentsRange(document), document, 'ComposedTree')");
shouldBeFalse("internals.rangeContainsNode(range(document, 0, document, 2), document, 'ComposedTree')");
shouldBeFalse("internals.rangeContainsNode(range(document, 0, documentElement, 0), body, 'ComposedTree')");
shouldBeFalse("internals.rangeContainsNode(range(document, 0, documentElement, 1), body, 'ComposedTree')");
shouldBeFalse("internals.rangeContainsNode(range(document, 0, body, 0), body, 'ComposedTree')");
shouldBeFalse("internals.rangeContainsNode(range(document, 0, body, 1), body, 'ComposedTree')");
shouldBeTrue("internals.rangeContainsNode(range(document, 0, documentElement, 2), body, 'ComposedTree')");

let a = document.createElement("div")
shouldBeFalse("internals.rangeContainsNode(contentsRange(document), a, 'ComposedTree')")

body.appendChild(a)

let b = document.createElement("div")
body.appendChild(b)

let c = document.createElement("div")
b.appendChild(c)

let d = document.createElement("div")
a.appendChild(d)

let e = document.createElement("div")
let f = document.createElement("div")
e.appendChild(f)
shouldBeFalse("internals.rangeContainsNode(contentsRange(body), f, 'ComposedTree')")

let g = document.createElement("textarea");
c.appendChild(g)
let h = internals.ensureUserAgentShadowRoot(g).firstChild
shouldBeTrue("internals.rangeContainsNode(contentsRange(body), h, 'ComposedTree')")

</script>
</body></html>
@@ -0,0 +1,42 @@
Test the contains function that checks if a range contains another range.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS internals.rangeContainsRange(contentsRange(document), contentsRange(document), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(documentElement), contentsRange(documentElement), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(body), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(document), contentsRange(documentElement), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(documentElement), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), contentsRange(body), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(documentElement), contentsRange(body), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(documentElement), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), contentsRange(documentElement), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(documentElement), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(documentElement), contentsRange(body), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(documentElement), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), collapsedRange(document, 0), 'ComposedTree') is true
PASS internals.rangeContainsRange(collapsedRange(document, 0), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), collapsedRange(document, 1), 'ComposedTree') is true
PASS internals.rangeContainsRange(collapsedRange(document, 1), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), collapsedRange(document, 2), 'ComposedTree') is false
PASS internals.rangeContainsRange(collapsedRange(document, 2), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(range(document, 0, document, 2), contentsRange(document), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(document), range(document, 0, document, 2), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), range(document, 1, document, 2), 'ComposedTree') is false
PASS internals.rangeContainsRange(range(document, 1, document, 2), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(range(document, 0, documentElement, 0), contentsRange(body), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(body), range(document, 0, documentElement, 0), 'ComposedTree') is false
PASS internals.rangeContainsRange(range(document, 0, body, 0), collapsedRange(body, 0), 'ComposedTree') is true
PASS internals.rangeContainsRange(collapsedRange(body, 0), range(document, 0, body, 0), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(document), contentsRange(a), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(a), contentsRange(document), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(f), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(f), contentsRange(body), 'ComposedTree') is false
PASS internals.rangeContainsRange(contentsRange(body), contentsRange(h), 'ComposedTree') is true
PASS internals.rangeContainsRange(contentsRange(h), contentsRange(body), 'ComposedTree') is false
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,89 @@
<html><head></head><body>
<script src="../../resources/js-test.js"></script>
<script>

description("Test the contains function that checks if a range contains another range.")

function range(startContainer, startOffset, endContainer, endOffset)
{
return new StaticRange({ startContainer: startContainer, startOffset: startOffset, endContainer: endContainer, endOffset: endOffset })
}

function collapsedRange(container, offset)
{
return range(container, offset, container, offset)
}

function contentsRange(node)
{
return range(node, 0, node, node.childNodes.length)
}

function shouldContainSelf(a)
{
shouldBeTrue("internals.rangeContainsRange(" + a + ", " + a + ", 'ComposedTree')")
}

function shouldBeNested(a, b)
{
shouldBeTrue("internals.rangeContainsRange(" + a + ", " + b + ", 'ComposedTree')")
shouldBeFalse("internals.rangeContainsRange(" + b + ", " + a + ", 'ComposedTree')")
}

function shouldNotContain(a, b)
{
shouldBeFalse("internals.rangeContainsRange(" + a + ", " + b + ", 'ComposedTree')")
shouldBeFalse("internals.rangeContainsRange(" + b + ", " + a + ", 'ComposedTree')")
}

let documentElement = document.documentElement;
let body = document.body;

shouldContainSelf("contentsRange(document)")
shouldContainSelf("contentsRange(documentElement)")
shouldContainSelf("contentsRange(body)")

shouldBeNested("contentsRange(document)", "contentsRange(documentElement)")
shouldBeNested("contentsRange(document)", "contentsRange(body)")
shouldBeNested("contentsRange(documentElement)", "contentsRange(body)")

shouldBeNested("contentsRange(document)", "contentsRange(documentElement)")

shouldBeNested("contentsRange(documentElement)", "contentsRange(body)")

shouldBeNested("contentsRange(document)", "collapsedRange(document, 0)")
shouldBeNested("contentsRange(document)", "collapsedRange(document, 1)")
shouldNotContain("contentsRange(document)", "collapsedRange(document, 2)")

shouldBeNested("range(document, 0, document, 2)", "contentsRange(document)")
shouldNotContain("contentsRange(document)", "range(document, 1, document, 2)")

shouldNotContain("range(document, 0, documentElement, 0)", "contentsRange(body)")
shouldBeNested("range(document, 0, body, 0)", "collapsedRange(body, 0)")

let a = document.createElement("div")
shouldNotContain("contentsRange(document)", "contentsRange(a)")

body.appendChild(a)

let b = document.createElement("div")
body.appendChild(b)

let c = document.createElement("div")
b.appendChild(c)

let d = document.createElement("div")
a.appendChild(d)

let e = document.createElement("div")
let f = document.createElement("div")
e.appendChild(f)
shouldNotContain("contentsRange(body)", "contentsRange(f)")

let g = document.createElement("textarea");
c.appendChild(g)
let h = internals.ensureUserAgentShadowRoot(g).firstChild
shouldBeNested("contentsRange(body)", "contentsRange(h)")

</script>
</body></html>
@@ -0,0 +1,59 @@
Test the tree ordering.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS internals.treeOrder(document, document, 'ComposedTree') is "equivalent"
PASS internals.treeOrder(body, body, 'ComposedTree') is "equivalent"
PASS internals.treeOrder(document, body, 'ComposedTree') is "less"
PASS internals.treeOrder(body, document, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, a, 'ComposedTree') is "equivalent"
PASS internals.treeOrder(body, a, 'ComposedTree') is "unordered"
PASS internals.treeOrder(a, body, 'ComposedTree') is "unordered"
PASS internals.treeOrder(body, a, 'ComposedTree') is "less"
PASS internals.treeOrder(a, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, a, 'ComposedTree') is "equivalent"
PASS internals.treeOrder(body, b, 'ComposedTree') is "less"
PASS internals.treeOrder(b, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, b, 'ComposedTree') is "less"
PASS internals.treeOrder(b, a, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, c, 'ComposedTree') is "less"
PASS internals.treeOrder(c, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, c, 'ComposedTree') is "less"
PASS internals.treeOrder(c, a, 'ComposedTree') is "greater"
PASS internals.treeOrder(b, c, 'ComposedTree') is "less"
PASS internals.treeOrder(c, b, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, d, 'ComposedTree') is "less"
PASS internals.treeOrder(d, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, d, 'ComposedTree') is "less"
PASS internals.treeOrder(d, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, d, 'ComposedTree') is "less"
PASS internals.treeOrder(d, a, 'ComposedTree') is "greater"
PASS internals.treeOrder(d, b, 'ComposedTree') is "less"
PASS internals.treeOrder(b, d, 'ComposedTree') is "greater"
PASS internals.treeOrder(d, c, 'ComposedTree') is "less"
PASS internals.treeOrder(c, d, 'ComposedTree') is "greater"
PASS internals.treeOrder(a, d, 'ComposedTree') is "less"
PASS internals.treeOrder(d, a, 'ComposedTree') is "greater"
PASS internals.treeOrder(d, b, 'ComposedTree') is "less"
PASS internals.treeOrder(b, d, 'ComposedTree') is "greater"
PASS internals.treeOrder(d, c, 'ComposedTree') is "less"
PASS internals.treeOrder(c, d, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, f, 'ComposedTree') is "unordered"
PASS internals.treeOrder(f, body, 'ComposedTree') is "unordered"
PASS internals.treeOrder(f, body, 'ComposedTree') is "unordered"
PASS internals.treeOrder(body, f, 'ComposedTree') is "unordered"
PASS internals.treeOrder(body, g, 'ComposedTree') is "less"
PASS internals.treeOrder(g, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, h, 'ComposedTree') is "less"
PASS internals.treeOrder(h, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(body, i, 'ComposedTree') is "less"
PASS internals.treeOrder(i, body, 'ComposedTree') is "greater"
PASS internals.treeOrder(i, d, 'ComposedTree') is "less"
PASS internals.treeOrder(d, i, 'ComposedTree') is "greater"
PASS internals.treeOrder(i, b, 'ComposedTree') is "less"
PASS internals.treeOrder(b, i, 'ComposedTree') is "greater"
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,85 @@
<html><body>
<script src="../../resources/js-test.js"></script>
<script>

description("Test the tree ordering.")

function shouldBeBoth(a, b, forward, reversed)
{
shouldBeEqualToString("internals.treeOrder(" + a + ", " + (b ? b : a) + ", 'ComposedTree')", forward)
if (a != b)
shouldBeEqualToString("internals.treeOrder(" + b + ", " + a + ", 'ComposedTree')", reversed)
}

function shouldBeEquivalent(a, b)
{
shouldBeBoth(a, b, "equivalent", "equivalent")
}

function shouldBeLess(a, b)
{
shouldBeBoth(a, b, "less", "greater")
}

function shouldBeUnordered(a, b)
{
shouldBeBoth(a, b, "unordered", "unordered")
}

shouldBeEquivalent("document", "document")

let body = document.body;

shouldBeEquivalent("body", "body")
shouldBeLess("document", "body")

let a = document.createElement("div")
shouldBeEquivalent("a", "a")
shouldBeUnordered("body", "a")

body.appendChild(a)
shouldBeLess("body", "a")
shouldBeEquivalent("a", "a")

let b = document.createElement("div")
body.appendChild(b)
shouldBeLess("body", "b")
shouldBeLess("a", "b")

let c = document.createElement("div")
b.appendChild(c)
shouldBeLess("body", "c")
shouldBeLess("a", "c")
shouldBeLess("b", "c")

let d = document.createElement("div")
a.appendChild(d)
shouldBeLess("body", "d")
shouldBeLess("body", "d")
shouldBeLess("a", "d")
shouldBeLess("d", "b")
shouldBeLess("d", "c")
shouldBeLess("a", "d")
shouldBeLess("d", "b")
shouldBeLess("d", "c")

let e = document.createElement("div")
let f = document.createElement("div")
e.appendChild(f)
shouldBeUnordered("body", "f")
shouldBeUnordered("f", "body")

let g = document.createElement("textarea")
c.appendChild(g)
shouldBeLess("body", "g")

let h = internals.ensureUserAgentShadowRoot(g).firstChild
shouldBeLess("body", "h")

let i = a.attachShadow({ mode: "closed" })
shouldBeLess("body", "i")
shouldBeLess("i", "d")
shouldBeLess("i", "b")

</script>
</body></html>

0 comments on commit 185f495

Please sign in to comment.