From 902edb925ede2c3d4a2c228aba8b01b9bb084e91 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Thu, 18 Jan 2018 11:57:22 +0100 Subject: [PATCH] Implement Element::has_css_layout_box() Calling scroll() on an element which is not rendered (by a parent with display: none) would previously cause a crash. In fact, we should terminate the algorithm [https://drafts.csswg.org/cssom-view/#dom-element-scroll] at step 10 in this situation. The fix hinges on implementing Element::has_css_layout_box() correctly, rather than just returning true in all cases as we did previously. Fixes #19430. --- components/script/dom/element.rs | 9 ++++---- tests/wpt/metadata/MANIFEST.json | 10 +++++++++ .../css/cssom-view/scroll-no-layout-box.html | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 tests/wpt/web-platform-tests/css/cssom-view/scroll-no-layout-box.html diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 30ac45802a8..1c884d0be94 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -349,11 +349,12 @@ impl Element { } // https://drafts.csswg.org/cssom-view/#css-layout-box - // Elements that have a computed value of the display property - // that is table-column or table-column-group - // FIXME: Currently, it is assumed to be true always + // + // We'll have no content box if there's no fragment for the node, and we use + // bounding_content_box, for simplicity, to detect this (rather than making a more specific + // query to the layout thread). fn has_css_layout_box(&self) -> bool { - true + self.upcast::().bounding_content_box().is_some() } // https://drafts.csswg.org/cssom-view/#potentially-scrollable diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 6c89d94229d..1e5937ddc0c 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -308101,6 +308101,12 @@ {} ] ], + "css/cssom-view/scroll-no-layout-box.html": [ + [ + "/css/cssom-view/scroll-no-layout-box.html", + {} + ] + ], "css/cssom-view/scrollIntoView-shadow.html": [ [ "/css/cssom-view/scrollIntoView-shadow.html", @@ -516726,6 +516732,10 @@ "966ebff69f91c0ea92f4bc2f943df9ef9dc3bcf9", "testharness" ], + "css/cssom-view/scroll-no-layout-box.html": [ + "8f423a757bd716c9c22e82fd2f35007d05982c08", + "testharness" + ], "css/cssom-view/scrollIntoView-shadow.html": [ "3c4a18992105fd7bf19cbf29f0b6d80cb12ca98c", "testharness" diff --git a/tests/wpt/web-platform-tests/css/cssom-view/scroll-no-layout-box.html b/tests/wpt/web-platform-tests/css/cssom-view/scroll-no-layout-box.html new file mode 100644 index 00000000000..cc67ce76ed9 --- /dev/null +++ b/tests/wpt/web-platform-tests/css/cssom-view/scroll-no-layout-box.html @@ -0,0 +1,21 @@ + + +cssom-view - Scrolling element with no layout box + + + + + +
+
+
+ +