CANCELLED Non-laid-out node should not be considered as user-select:contain

third_party/WebKit/Source/core/editing/EditingUtilities.cpp

Index: third_party/WebKit/Source/core/editing/EditingUtilities.cpp
diff --git a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
index 736d889bf286020e88b7afa6ef2f399bf276d8fd..960d4cc91252743b4ba05d1586aa9abb17ed0af9 100644
--- a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
+++ b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
@@ -298,6 +298,8 @@ int comparePositions(const VisiblePosition& a, const VisiblePosition& b) {
 // TODO(editing-dev): We should make |SelectionAdjuster| to use this funciton
 // instead of |isSelectionBondary()|.
 bool isUserSelectContain(const Node& node) {
+  if (!node.layoutObject())
+    return false;

[yoichio, 2017/03/21 06:48:43]

We want |isUserSelectContain| independent from layout.
Could you check sanity in callers?

[yosin_UTC9, 2017/03/21 08:48:51]

Fumm... It seems we want to introduce isDisplayHiddenOf(const Node&)

bool isDisplayHidden(const Node& node) {
  DCHECK(node.isConnected()) << node;
  DCHECK_GE(node.document().lifecylce().state(), DocumentLifecycle::StyleClean);
  ComputedStyle* style = node->computedStyle();
  // |style| can be null if
  // - |node| isn't element, and
  // - TBD: Not sure when computed style of non-element |node| is null.
  return style && style->display() == EDisplay::KNone;
}

Just introduce limited version of isDisplayNonde(const Node&)? Like:

isDisplayNone(const Node& node) {
  DCHECK(node.isConnected());
  DCEHCK(style clean);
  return !node.layoutObject();
}


BTW, display and user-select are orthogonal, we should introduce another
function e.g. shouldSkipNode(const Node&), or another good name
to check isDisplayNonde() and isUserSelectContent()

[Xiaocheng, 2017/03/21 20:27:48]

I'm confused.

What should be the ultimate fix to this bug?

Declare that a node without parent in flat tree cannot be the anchor node of a
Before/AfterAnchor PositionInFlatTree, and stop callers from generating such
positions?

Or add more checks in the callers (for this bug,
adjustPositionForBackwardIteration)?

[yosin_UTC9, 2017/03/22 03:19:28]

Change canonicalization not to return a position in shadow host:
  <div id="host"><a>foo</a><b>bar</b></div>
Invalid: P(host, N), {After,Before}Children(host), {After,Before}Anchor(A),
{After,Before}Anchor(B)
Valid: P("foo", N), {After,Before}Children(A), {After,Before}Children(B)
Yes.
We should not pass a position in shadow host to mostBackwardCaretPosition().

   return isHTMLTextAreaElement(node) || isHTMLInputElement(node) ||
          isHTMLSelectElement(node);
 }