DL: Ensure to not not process innerText queries on locked subtrees.
This patch skips innerText processing for elements that are either
display locked, or are inside a locked subtree.
This is due to the fact that display locked subtrees are not painted
and so should behave in the same way as visibility hidden (or display
none elements).
This also ensures that we don't access text nodes when they have not
been laid out, which is possible in display locking. This causes
CHECKs with LayoutNG.
R=chrishtr@chromium.org, rakina@chromium.org, xiaochengh@chromium.org
Bug: 962569
Change-Id: I0f286b7ea55e72baada3d9f2c10f4ea693e268d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1609666
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659664}
diff --git a/third_party/blink/renderer/core/display_lock/display_lock_utilities.cc b/third_party/blink/renderer/core/display_lock/display_lock_utilities.cc
index 11d0f45..0e2dbe5 100644
--- a/third_party/blink/renderer/core/display_lock/display_lock_utilities.cc
+++ b/third_party/blink/renderer/core/display_lock/display_lock_utilities.cc
@@ -80,7 +80,8 @@
}
}
-Element* DisplayLockUtilities::NearestLockedInclusiveAncestor(Node& node) {
+const Element* DisplayLockUtilities::NearestLockedInclusiveAncestor(
+ const Node& node) {
if (!RuntimeEnabledFeatures::DisplayLockingEnabled() ||
node.GetDocument().LockedDisplayLockCount() == 0 ||
!node.CanParticipateInFlatTree()) {
@@ -95,6 +96,11 @@
return NearestLockedExclusiveAncestor(node);
}
+Element* DisplayLockUtilities::NearestLockedInclusiveAncestor(Node& node) {
+ return const_cast<Element*>(
+ NearestLockedInclusiveAncestor(static_cast<const Node&>(node)));
+}
+
Element* DisplayLockUtilities::NearestLockedExclusiveAncestor(
const Node& node) {
if (!RuntimeEnabledFeatures::DisplayLockingEnabled() ||
diff --git a/third_party/blink/renderer/core/display_lock/display_lock_utilities.h b/third_party/blink/renderer/core/display_lock/display_lock_utilities.h
index 20b5989..c1d0c08 100644
--- a/third_party/blink/renderer/core/display_lock/display_lock_utilities.h
+++ b/third_party/blink/renderer/core/display_lock/display_lock_utilities.h
@@ -44,6 +44,7 @@
Element& element);
// Returns the nearest inclusive ancestor of |node| that is display locked.
+ static const Element* NearestLockedInclusiveAncestor(const Node& node);
static Element* NearestLockedInclusiveAncestor(Node& node);
// Returns the nearest non-inclusive ancestor of |node| that is display
diff --git a/third_party/blink/renderer/core/editing/element_inner_text.cc b/third_party/blink/renderer/core/editing/element_inner_text.cc
index 1979656..e9d234b2 100644
--- a/third_party/blink/renderer/core/editing/element_inner_text.cc
+++ b/third_party/blink/renderer/core/editing/element_inner_text.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include "base/auto_reset.h"
+#include "third_party/blink/renderer/core/display_lock/display_lock_utilities.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/dom/node_traversal.h"
#include "third_party/blink/renderer/core/dom/text.h"
@@ -94,7 +95,13 @@
String ElementInnerTextCollector::RunOn(const Element& element) {
DCHECK(!element.InActiveDocument() || !NeedsLayoutTreeUpdate(element));
- // 1. If this element is not being rendered, or if the user agent is a non-CSS
+ // 1. If this element is locked or a part of a locked subtree, then it is
+ // hidden from view (and also possibly not laid out) and innerText should be
+ // empty.
+ if (DisplayLockUtilities::NearestLockedInclusiveAncestor(element))
+ return {};
+
+ // 2. If this element is not being rendered, or if the user agent is a non-CSS
// user agent, then return the same value as the textContent IDL attribute on
// this element.
// Note: To pass WPT test, case we don't use |textContent| for
@@ -106,8 +113,8 @@
return element.textContent(convert_brs_to_newlines);
}
- // 2. Let results be a new empty list.
- // 3. For each child node node of this element:
+ // 3. Let results be a new empty list.
+ // 4. For each child node node of this element:
// 1. Let current be the list resulting in running the inner text collection
// steps with node. Each item in results will either be a JavaScript
// string or a positive integer (a required line break count).
@@ -260,13 +267,21 @@
// each child node of node in tree order, and then concatenating the results
// to a single list.
- // 2. If node's computed value of 'visibility' is not 'visible', then return
+ // 2. If the node is display locked, then we should not process it or its
+ // children, since they are not visible or accessible via innerText.
+ if (node.IsElementNode()) {
+ auto* context = ToElement(node).GetDisplayLockContext();
+ if (context && context->IsLocked())
+ return;
+ }
+
+ // 3. If node's computed value of 'visibility' is not 'visible', then return
// items.
const ComputedStyle* style = node.GetComputedStyle();
if (style && style->Visibility() != EVisibility::kVisible)
return ProcessChildren(node);
- // 3. If node is not being rendered, then return items. For the purpose of
+ // 4. If node is not being rendered, then return items. For the purpose of
// this step, the following elements must act as described if the computed
// value of the 'display' property is not 'none':
// Note: items can be non-empty due to 'display:contents'.
@@ -290,12 +305,12 @@
return ProcessOptionElement(ToHTMLOptionElement(node));
}
- // 4. If node is a Text node, then for each CSS text box produced by node.
+ // 5. If node is a Text node, then for each CSS text box produced by node.
auto* text_node = DynamicTo<Text>(node);
if (text_node)
return ProcessTextNode(*text_node);
- // 5. If node is a br element, then append a string containing a single U+000A
+ // 6. If node is a br element, then append a string containing a single U+000A
// LINE FEED (LF) character to items.
if (IsHTMLBRElement(node)) {
ProcessChildren(node);
@@ -303,7 +318,7 @@
return;
}
- // 6. If node's computed value of 'display' is 'table-cell', and node's CSS
+ // 7. If node's computed value of 'display' is 'table-cell', and node's CSS
// box is not the last 'table-cell' box of its enclosing 'table-row' box, then
// append a string containing a single U+0009 CHARACTER TABULATION (tab)
// character to items.
@@ -316,7 +331,7 @@
return;
}
- // 7. If node's computed value of 'display' is 'table-row', and node's CSS box
+ // 8. If node's computed value of 'display' is 'table-row', and node's CSS box
// is not the last 'table-row' box of the nearest ancestor 'table' box, then
// append a string containing a single U+000A LINE FEED (LF) character to
// items.
@@ -328,7 +343,7 @@
return;
}
- // 8. If node is a p element, then append 2 (a required line break count) at
+ // 9. If node is a p element, then append 2 (a required line break count) at
// the beginning and end of items.
if (IsHTMLParagraphElement(node)) {
// Note: <p style="display:contents>foo</p> doesn't generate layout object
@@ -337,7 +352,7 @@
return;
}
- // 9. If node's used value of 'display' is block-level or 'table-caption',
+ // 10. If node's used value of 'display' is block-level or 'table-caption',
// then append 1 (a required line break count) at the beginning and end of
// items.
if (IsDisplayBlockLevel(node))
diff --git a/third_party/blink/web_tests/wpt_internal/display-lock/lock-after-append/inner-text.html b/third_party/blink/web_tests/wpt_internal/display-lock/lock-after-append/inner-text.html
new file mode 100644
index 0000000..2a06aec
--- /dev/null
+++ b/third_party/blink/web_tests/wpt_internal/display-lock/lock-after-append/inner-text.html
@@ -0,0 +1,34 @@
+<!doctype HTML>
+<html>
+<meta charset="utf8">
+<title>Display Locking: innerText</title>
+<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
+<link rel="help" href="https://github.com/WICG/display-locking">
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<style>
+#container {
+ contain: style layout;
+}
+</style>
+
+This text should be visible.
+<div id="container">
+ This text should not be visible.
+ <div id="inner">
+ This text is also not visible.
+ </div>
+</div>
+
+<script>
+promise_test(async () => {
+ const container = document.getElementById("container");
+ await container.displayLock.acquire({ timeout: Infinity });
+
+ assert_equals(document.body.innerText, "This text should be visible.");
+ assert_equals(document.getElementById("inner").innerText, "");
+}, "innerText on locked element.");
+</script>
+</html>
