Attachment #159873 for bug #92899

View Details Raw Unified Return to Bug 92899

| Differences between



2012-08-17  Takashi Sakamoto  <tasak@google.com>

        Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.
        https://bugs.webkit.org/show_bug.cgi?id=92899

        Reviewed by Hajime Morita.

        Since childNeedsStyleRecalc is not cleared when parent nodes are
        attached, setNeedsStyleRecalc flag is not reached Document. So,
        document() doesn't run re-layout.

        Test: fast/dom/shadow/shadowdom-dynamic-styling.html

        * dom/ContainerNode.h:
        (ContainerNode):
        (WebCore::ContainerNode::detachAsNode):
        Removed detachAsNode, because the below change made the method
        not-used.
        * dom/Element.cpp:
        (WebCore::Element::detach):
        Modify to invoke ContainerNode::detach when any shadow subtree is
        attached. ContainerNode::detach takes care of childNeedsStyleRecalc
        flag.
        * dom/ElementShadow.cpp:
        (WebCore::ElementShadow::invalidateDistribution):
        Use SetAttached for lazyAttach instead of DoNotSetAttached, because
        it is reuired to invoke ContainerNode::detach. If not, attached() is
        false and reattach() invokes only attach(). This causes to leave
        shadow host's childNeedsStyleRecalc flag true after
        Element::recalcStyle.

2012-08-21  Kentaro Hara  <haraken@chromium.org>

        [V8] Move String related code in V8Binding to a separate file



    void attachChildren();
    void attachChildrenIfNeeded();
    void attachChildrenLazily();
    void detachAsNode();
    void detachChildren();
    void detachChildrenIfNeeded();


            child->lazyAttach();
}

inline void ContainerNode::detachAsNode()
{
    Node::detach();
}

inline void ContainerNode::detachChildrenIfNeeded()
{
    for (Node* child = firstChild(); child; child = child->nextSibling()) {



    if (ElementShadow* shadow = this->shadow()) {
        detachChildrenIfNeeded();
        shadow->detach();
    }
    ContainerNode::detach();
        ContainerNode::detach();

    RenderWidget::resumeWidgetHierarchyUpdates();
}

Lines 206-212 void ElementShadow::invalidateDistribution(Element* host) a/Source/WebCore/dom/ElementShadow.cpp_sec1
- a/Source/WebCore/dom/ElementShadow.cpp -1 / +1 lines
206		206
207	if (needsReattach && host->attached()) {	207	if (needsReattach && host->attached()) {
208	host->detach();	208	host->detach();
209	host->lazyAttach(Node::DoNotSetAttached);	209	host->lazyAttach();
210	}	210	}
211		211
212	if (needsInvalidation)	212	if (needsInvalidation)



2012-08-17  Takashi Sakamoto  <tasak@google.com>

        Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.
        https://bugs.webkit.org/show_bug.cgi?id=92899

        Reviewed by Hajime Morita.

        * fast/dom/shadow/shadowdom-dynamic-styling-expected.txt: Added.
        * fast/dom/shadow/shadowdom-dynamic-styling.html: Added.
        * editing/shadow/delete-characters-in-distributed-node-crash.html:
        Made the layout test robust. This patch causes commit-queue- and the
        failed test is delete-characters-in-distributed-node-crash.html.
        However the layout test passes when locally running run_webkit_test
        i.e. no difference between actual and expected texts. And looking at
        the commit-queue's report, the actual text has "PASS" message (no
        crash messages).

2012-08-21  Dominic Cooney  <dominicc@chromium.org>

        [Chromium] Unreviewed gardening.

Lines 1-4 a/LayoutTests/editing/shadow/delete-characters-in-distributed-node-crash-expected.txt_sec1
- a/LayoutTests/editing/shadow/delete-characters-in-distributed-node-crash-expected.txt -1 lines
1	This tests the deletion of text in distributed node does not crash. To run it outside of DRT, you must delete text, 'foo', manually.	1	This tests the deletion of text in distributed node does not crash. To run it outside of DRT, you must delete text, 'foo', manually.
2		2
3
4	PASS	3	PASS



<html>
<body>
<p id="description">This tests the deletion of text in distributed node does not crash. To run it outside of DRT, you must delete text, 'foo', manually.</p>
<div id="wrapper"><div id="shadowhost" contenteditable><div>foo</div></div></div>
<script>
if (window.testRunner)
    testRunner.dumpAsText();

window.getSelection().setBaseAndExtent(textNode, 0, textNode, 3);
document.execCommand('Delete');

document.getElementById("wrapper").innerHTML = "PASS";
</script>
</body>
</html>



Test for https://bugs.webkit.org/show_bug.cgi?id=92899. Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.

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


PASS window.getComputedStyle(target).backgroundColor is "rgb(255, 0, 0)"
PASS successfullyParsed is true

TEST COMPLETE




<!doctype html>
<html>
<head>
  <script src="../../js/resources/js-test-pre.js"></script>
  <script>
description("Test for https://bugs.webkit.org/show_bug.cgi?id=92899. Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.");

if (window.testRunner)
   testRunner.dumpAsText();

</script>
</head>
<body>
  <div id="box" style="border: 1px solid black;">
    <div id="item"><div>Content required here to reproduce bug.</div></div>
  </div>
<script>
var box = document.querySelector("#box");
var oldestShadowRoot = new WebKitShadowRoot(box);
oldestShadowRoot.innerHTML = "<content></content>";
    
var youngestShadowRoot = new WebKitShadowRoot(document.querySelector("#item"));
youngestShadowRoot.innerHTML = "<style>" +
      ".selected {background: red;}" +
      "</style>" +
      "<div id='target'>Content</div>";

document.body.offsetLeft;
    
target = youngestShadowRoot.querySelector("div");
target.classList.add("selected");
shouldBe('window.getComputedStyle(target).backgroundColor', '"rgb(255, 0, 0)"');
</script>
<script src="../../js/resources/js-test-post.js"></script>
</body>
</html>

Return to Bug 92899

Lines 92-98 public: a/Source/WebCore/dom/ContainerNode.h_sec1
- a/Source/WebCore/dom/ContainerNode.h -6 lines
92	void attachChildren();	92	void attachChildren();
93	void attachChildrenIfNeeded();	93	void attachChildrenIfNeeded();
94	void attachChildrenLazily();	94	void attachChildrenLazily();
95	void detachAsNode();
96	void detachChildren();	95	void detachChildren();
97	void detachChildrenIfNeeded();	96	void detachChildrenIfNeeded();
98		97
Lines 188-198 inline void ContainerNode::attachChildrenLazily() a/Source/WebCore/dom/ContainerNode.h_sec2
188	child->lazyAttach();	187	child->lazyAttach();
189	}	188	}
190		189
191	inline void ContainerNode::detachAsNode()
192	{
193	Node::detach();
194	}
195
196	inline void ContainerNode::detachChildrenIfNeeded()	190	inline void ContainerNode::detachChildrenIfNeeded()
197	{	191	{
198	for (Node* child = firstChild(); child; child = child->nextSibling()) {	192	for (Node* child = firstChild(); child; child = child->nextSibling()) {

Lines 999-1007 void Element::detach() a/Source/WebCore/dom/Element.cpp_sec1
- a/Source/WebCore/dom/Element.cpp -3 / +2 lines
999	if (ElementShadow* shadow = this->shadow()) {	999	if (ElementShadow* shadow = this->shadow()) {
1000	detachChildrenIfNeeded();	1000	detachChildrenIfNeeded();
1001	shadow->detach();	1001	shadow->detach();
1002	detachAsNode();	1002	}
1003	} else	1003	ContainerNode::detach();
1004	ContainerNode::detach();
1005		1004
1006	RenderWidget::resumeWidgetHierarchyUpdates();	1005	RenderWidget::resumeWidgetHierarchyUpdates();
1007	}	1006	}

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +31 lines
		1	2012-08-17 Takashi Sakamoto <tasak@google.com>
		2
		3	Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.
		4	https://bugs.webkit.org/show_bug.cgi?id=92899
		5
		6	Reviewed by Hajime Morita.
		7
		8	Since childNeedsStyleRecalc is not cleared when parent nodes are
		9	attached, setNeedsStyleRecalc flag is not reached Document. So,
		10	document() doesn't run re-layout.
		11
		12	Test: fast/dom/shadow/shadowdom-dynamic-styling.html
		13
		14	* dom/ContainerNode.h:
		15	(ContainerNode):
		16	(WebCore::ContainerNode::detachAsNode):
		17	Removed detachAsNode, because the below change made the method
		18	not-used.
		19	* dom/Element.cpp:
		20	(WebCore::Element::detach):
		21	Modify to invoke ContainerNode::detach when any shadow subtree is
		22	attached. ContainerNode::detach takes care of childNeedsStyleRecalc
		23	flag.
		24	* dom/ElementShadow.cpp:
		25	(WebCore::ElementShadow::invalidateDistribution):
		26	Use SetAttached for lazyAttach instead of DoNotSetAttached, because
		27	it is reuired to invoke ContainerNode::detach. If not, attached() is
		28	false and reattach() invokes only attach(). This causes to leave
		29	shadow host's childNeedsStyleRecalc flag true after
		30	Element::recalcStyle.
		31
1	2012-08-21 Kentaro Hara <haraken@chromium.org>	32	2012-08-21 Kentaro Hara <haraken@chromium.org>
2		33
3	[V8] Move String related code in V8Binding to a separate file	34	[V8] Move String related code in V8Binding to a separate file

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +17 lines
		1	2012-08-17 Takashi Sakamoto <tasak@google.com>
		2
		3	Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.
		4	https://bugs.webkit.org/show_bug.cgi?id=92899
		5
		6	Reviewed by Hajime Morita.
		7
		8	* fast/dom/shadow/shadowdom-dynamic-styling-expected.txt: Added.
		9	* fast/dom/shadow/shadowdom-dynamic-styling.html: Added.
		10	* editing/shadow/delete-characters-in-distributed-node-crash.html:
		11	Made the layout test robust. This patch causes commit-queue- and the
		12	failed test is delete-characters-in-distributed-node-crash.html.
		13	However the layout test passes when locally running run_webkit_test
		14	i.e. no difference between actual and expected texts. And looking at
		15	the commit-queue's report, the actual text has "PASS" message (no
		16	crash messages).
		17
1	2012-08-21 Dominic Cooney <dominicc@chromium.org>	18	2012-08-21 Dominic Cooney <dominicc@chromium.org>
2		19
3	[Chromium] Unreviewed gardening.	20	[Chromium] Unreviewed gardening.

Lines 2-8 a/LayoutTests/editing/shadow/delete-characters-in-distributed-node-crash.html_sec1
- a/LayoutTests/editing/shadow/delete-characters-in-distributed-node-crash.html -2 / +2 lines
2	<html>	2	<html>
3	<body>	3	<body>
4	<p id="description">This tests the deletion of text in distributed node does not crash. To run it outside of DRT, you must delete text, 'foo', manually.</p>	4	<p id="description">This tests the deletion of text in distributed node does not crash. To run it outside of DRT, you must delete text, 'foo', manually.</p>
5	<div id="shadowhost" contenteditable><div>foo</div></div>	5	<div id="wrapper"><div id="shadowhost" contenteditable><div>foo</div></div></div>
6	<script>	6	<script>
7	if (window.testRunner)	7	if (window.testRunner)
8	testRunner.dumpAsText();	8	testRunner.dumpAsText();
Lines 13-19 var textNode = document.getElementById('shadowhost').firstChild.firstChild; a/LayoutTests/editing/shadow/delete-characters-in-distributed-node-crash.html_sec2
13	window.getSelection().setBaseAndExtent(textNode, 0, textNode, 3);	13	window.getSelection().setBaseAndExtent(textNode, 0, textNode, 3);
14	document.execCommand('Delete');	14	document.execCommand('Delete');
15		15
16	document.write('PASS');	16	document.getElementById("wrapper").innerHTML = "PASS";
17	</script>	17	</script>
18	</body>	18	</body>
19	</html>	19	</html>

Line 0 a/LayoutTests/fast/dom/shadow/shadowdom-dynamic-styling-expected.txt_sec1
- a/LayoutTests/fast/dom/shadow/shadowdom-dynamic-styling-expected.txt +10 lines
	1	Test for https://bugs.webkit.org/show_bug.cgi?id=92899. Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.
	2
	3	On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
	4
	5
	6	PASS window.getComputedStyle(target).backgroundColor is "rgb(255, 0, 0)"
	7	PASS successfullyParsed is true
	8
	9	TEST COMPLETE
	10

Line 0 a/LayoutTests/fast/dom/shadow/shadowdom-dynamic-styling.html_sec1
- a/LayoutTests/fast/dom/shadow/shadowdom-dynamic-styling.html +36 lines
	1	<!doctype html>
	2	<html>
	3	<head>
	4	<script src="../../js/resources/js-test-pre.js"></script>
	5	<script>
	6	description("Test for https://bugs.webkit.org/show_bug.cgi?id=92899. Dynamically styling ShadowDom content on a node distributed to another shadow insertion point fails.");
	7
	8	if (window.testRunner)
	9	testRunner.dumpAsText();
	10
	11	</script>
	12	</head>
	13	<body>
	14	<div id="box" style="border: 1px solid black;">
	15	<div id="item"><div>Content required here to reproduce bug.</div></div>
	16	</div>
	17	<script>
	18	var box = document.querySelector("#box");
	19	var oldestShadowRoot = new WebKitShadowRoot(box);
	20	oldestShadowRoot.innerHTML = "<content></content>";
	21
	22	var youngestShadowRoot = new WebKitShadowRoot(document.querySelector("#item"));
	23	youngestShadowRoot.innerHTML = "<style>" +
	24	".selected {background: red;}" +
	25	"</style>" +
	26	"<div id='target'>Content</div>";
	27
	28	document.body.offsetLeft;
	29
	30	target = youngestShadowRoot.querySelector("div");
	31	target.classList.add("selected");
	32	shouldBe('window.getComputedStyle(target).backgroundColor', '"rgb(255, 0, 0)"');
	33	</script>
	34	<script src="../../js/resources/js-test-post.js"></script>
	35	</body>
	36	</html>