Commit 77b4785e authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

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: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659664}
parent 5c6d022d
......@@ -80,7 +80,8 @@ DisplayLockUtilities::ScopedChainForcedUpdate::ScopedChainForcedUpdate(
}
}
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 @@ Element* DisplayLockUtilities::NearestLockedInclusiveAncestor(Node& node) {
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() ||
......
......@@ -44,6 +44,7 @@ class CORE_EXPORT DisplayLockUtilities {
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
......
......@@ -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 @@ class ElementInnerTextCollector final {
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 @@ String ElementInnerTextCollector::RunOn(const Element& element) {
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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
// 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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
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 @@ void ElementInnerTextCollector::ProcessNode(const Node& node) {
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))
......
<!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>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment