Elements must be reattached when inserted/removed from top layer
authorfalken@chromium.org <falken@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2013 04:46:21 +0000 (04:46 +0000)
committerfalken@chromium.org <falken@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jan 2013 04:46:21 +0000 (04:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105489

Reviewed by Julien Chaffraix.

Source/WebCore:

Ensure a reattach occurs when an element is inserted/removed from top layer, so its renderer can be inserted correctly:
as a child of RenderView in top layer sibling order if it's in the top layer, and in the usual place otherwise.

We previously relied on style recalc to catch when an element is inserted/removed from the top layer, because it
only happens on dialog.show/close which toggle display: none. But that is incorrect because, for example, close()
followed immediately by show() results in no style change.

Tests: fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html
       fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html

* dom/Element.cpp:
(WebCore::Element::removedFrom): Call Document::removeFromTopLayer to let the element be removed from the top layer vector.
removeFromTopLayer calls Element::setIsInTopLayer(false) itself if needed.
(WebCore::Element::setIsInTopLayer): Ensure a reattach occurs if the element is already attached.

LayoutTests:

* fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html: Added.
* fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html: Added.
This tests that a top layer element removed from the document does not reappear in the top layer if readded.
This test actually would pass before this patch, but just by good fortune (see bug).
* fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html: Added.
* fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html: Added.
This tests that top layer ordering is correct after removing and readding an element to the top layer.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139402 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp

index 5d1b60a..e4a5482 100644 (file)
@@ -1,3 +1,18 @@
+2013-01-10  Matt Falkenhagen  <falken@chromium.org>
+
+        Elements must be reattached when inserted/removed from top layer
+        https://bugs.webkit.org/show_bug.cgi?id=105489
+
+        Reviewed by Julien Chaffraix.
+
+        * fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html: Added.
+        * fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html: Added.
+        This tests that a top layer element removed from the document does not reappear in the top layer if readded.
+        This test actually would pass before this patch, but just by good fortune (see bug).
+        * fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html: Added.
+        * fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html: Added.
+        This tests that top layer ordering is correct after removing and readding an element to the top layer.
+
 2013-01-10  Shinya Kawanaka  <shinyak@chromium.org>
 
         When a selected node in nested ShadowDOM is deleted, selection have wrong range.
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html b/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html
new file mode 100644 (file)
index 0000000..219b156
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setDialogElementEnabled(true);
+</script>
+<style>
+.pseudodialog {
+    position: absolute;
+    left: 0; right: 0;
+    margin: auto;
+    border: solid;
+    padding: 1em;
+    background: white;
+    color: black;
+    height: 100px;
+    width: 100px;
+}
+
+#bottomDialog {
+    background-color: blue;
+    top: 100px;
+}
+
+#topDialog {
+    background-color: green;
+    top: 150px;
+    left: 50px;
+}
+</style>
+</head>
+<body>
+<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
+<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
+<div id="bottomDialog" class="pseudodialog"></div>
+<div id="topDialog" class="pseudodialog"></div>
+</body>
+</html>
+
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html b/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html
new file mode 100644 (file)
index 0000000..a6cd37d
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setDialogElementEnabled(true);
+</script>
+<style>
+dialog {
+    height: 100px;
+    width: 100px;
+}
+
+#bottomDialog {
+    background-color: blue;
+    top: 100px;
+}
+
+#topDialog {
+    background-color: green;
+    top: 150px;
+    left: 50px;
+}
+</style>
+</head>
+<body>
+<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
+<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
+<dialog id="bottomDialog"></dialog>
+<dialog id="topDialog"></dialog>
+<script>
+document.getElementById('topDialog').showModal();
+var bottomDialog = document.getElementById('bottomDialog');
+bottomDialog.showModal();
+bottomDialog.offsetTop;  // force a layout
+var parent = bottomDialog.parentNode;
+parent.removeChild(bottomDialog);
+parent.appendChild(bottomDialog);
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html b/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html
new file mode 100644 (file)
index 0000000..b2900d2
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setDialogElementEnabled(true);
+</script>
+<style>
+.pseudodialog {
+    height: 100px;
+    width: 100px;
+    position: absolute;
+    left: 0; right: 0;
+    margin: auto;
+    border: solid;
+    padding: 1em;
+    background: white;
+    color: black;
+}
+</style>
+</head>
+<body>
+<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
+<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
+
+<div class="pseudodialog" style="top: 100px; background-color: blue"></div>
+<div class="pseudodialog" style="top: 150px; left: 50px; background-color: green"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html b/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html
new file mode 100644 (file)
index 0000000..87694ce
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setDialogElementEnabled(true);
+</script>
+<style>
+dialog {
+    height: 100px;
+    width: 100px;
+}
+
+#bottomDialog {
+    background-color: blue;
+    top: 100px;
+}
+
+#topDialog {
+    background-color: green;
+    top: 150px;
+    left: 50px;
+}
+</style>
+</head>
+<body>
+<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
+<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
+
+<dialog id="topDialog"></dialog>
+<dialog id="bottomDialog"></dialog>
+<script>
+var topDialog = document.getElementById('topDialog');
+var bottomDialog = document.getElementById('bottomDialog');
+topDialog.showModal();
+bottomDialog.showModal();
+topDialog.offsetTop;  // force a layout
+topDialog.close();
+topDialog.showModal();
+</script>
+</body>
+</html>
index 9c52348..b412633 100644 (file)
@@ -1,3 +1,25 @@
+2013-01-10  Matt Falkenhagen  <falken@chromium.org>
+
+        Elements must be reattached when inserted/removed from top layer
+        https://bugs.webkit.org/show_bug.cgi?id=105489
+
+        Reviewed by Julien Chaffraix.
+
+        Ensure a reattach occurs when an element is inserted/removed from top layer, so its renderer can be inserted correctly:
+        as a child of RenderView in top layer sibling order if it's in the top layer, and in the usual place otherwise.
+
+        We previously relied on style recalc to catch when an element is inserted/removed from the top layer, because it
+        only happens on dialog.show/close which toggle display: none. But that is incorrect because, for example, close()
+        followed immediately by show() results in no style change.
+
+        Tests: fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html
+               fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::removedFrom): Call Document::removeFromTopLayer to let the element be removed from the top layer vector.
+        removeFromTopLayer calls Element::setIsInTopLayer(false) itself if needed.
+        (WebCore::Element::setIsInTopLayer): Ensure a reattach occurs if the element is already attached.
+
 2013-01-10  Shinya Kawanaka  <shinyak@chromium.org>
 
         When a selected node in nested ShadowDOM is deleted, selection have wrong range.
index 4934b9d..a311368 100644 (file)
@@ -1139,7 +1139,7 @@ void Element::removedFrom(ContainerNode* insertionPoint)
 #endif
 
 #if ENABLE(DIALOG_ELEMENT)
-    setIsInTopLayer(false);
+    document()->removeFromTopLayer(this);
 #endif
 #if ENABLE(FULLSCREEN_API)
     if (containsFullScreenElement())
@@ -2374,7 +2374,10 @@ bool Element::isInTopLayer() const
 void Element::setIsInTopLayer(bool inTopLayer)
 {
     ensureElementRareData()->setIsInTopLayer(inTopLayer);
-    setNeedsStyleRecalc(SyntheticStyleChange);
+
+    // We must ensure a reattach occurs so the renderer is inserted in the correct sibling order under RenderView according to its
+    // top layer position, or in its usual place if not in the top layer.
+    reattachIfAttached();
 }
 #endif