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

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

 /*
  * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 280 matching lines @@
 
 int comparePositions(const VisiblePosition& a, const VisiblePosition& b) {
   return comparePositions(a.deepEquivalent(), b.deepEquivalent());
 }
 
 // TODO(editing-dev): We should implement real version which refers
 // "user-select" CSS property.
 // 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);
 }
 
 enum EditableLevel { Editable, RichlyEditable };
 static bool hasEditableLevel(const Node& node, EditableLevel editableLevel) {
   DCHECK(node.document().isActive());
   // TODO(editing-dev): We should have this check:
   // DCHECK_GE(node.document().lifecycle().state(),
   //           DocumentLifecycle::StyleClean);
@@ skipping 1862 matching lines @@
         return InputType::DeleteWordBackward;
       if (granularity == LineBoundary)
         return InputType::DeleteLineBackward;
       return InputType::DeleteContentBackward;
     default:
       return InputType::None;
   }
 }
 
 }  // namespace blink