Canonicalization should avoid positions in shadow hosts |
|||
Issue description
Repro case is the following unit test:
TEST_F(VisibleUnitsTest, canonicalizeSelectOutOfFlatTree) {
setBodyContent("<div id=host><select></select></div>");
setShadowContent("foo", "host");
Element* select = document().querySelector("select");
EXPECT_EQ(PositionInFlatTree(), canonicalPositionOf(PositionInFlatTree::afterNode(select)));
}
The following DCHECK is hit:
[30460:30460:0317/130143.439929:1791738843018:FATAL:Position.cpp(123)] Check failed: offset == 0 (1 vs. 0)
#0 0x7ff0d292fccb base::debug::StackTrace::StackTrace()
#1 0x7ff0d292e35c base::debug::StackTrace::StackTrace()
#2 0x7ff0d299c26f logging::LogMessage::~LogMessage()
#3 0x7ff0ce851bb2 blink::PositionTemplate<>::PositionTemplate()
#4 0x7ff0ce8523e7 blink::PositionTemplate<>::toOffsetInAnchor()
#5 0x7ff0ce89132b blink::adjustPositionForBackwardIteration<>()
#6 0x7ff0ce88856b blink::mostBackwardCaretPosition<>()
#7 0x7ff0ce888322 blink::mostBackwardCaretPosition()
#8 0x7ff0ce87978d blink::canonicalPosition<>()
#9 0x7ff0ce87949c blink::canonicalPositionOf()
,
Mar 22 2017
As discussed in crrev.com/2760533004, the ultimate fix is to make canonicalization avoid DOM positions in shadow hosts (SELECT@AfterAnchor in this case). Example: <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) As we are trying to rebuild canonicalization upon LayoutNG, we do not want to introduce more hacky code to the current canonicalization system
,
Mar 23 2017
Once we don't pass immediate child nodes to canonicalizePositionOf(), we don't need to case the case of #c1.
,
Apr 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0cbb9fc2999813a837b8ee0483f77e3879cc25e0 commit 0cbb9fc2999813a837b8ee0483f77e3879cc25e0 Author: xiaochengh <xiaochengh@chromium.org> Date: Thu Apr 27 15:04:10 2017 Stop flat tree selection canonicalization from using invalid positions There are some valid DOM positions (*) that do not have corresponding valid flat tree positions. This patch adds special handling of such DOM positions, so that when computing VisibleSelectionInFlatTree from SelectionInDOMTree, such positions are converted to NULL instead of invalid flat tree positions, so that the renderer does not crash. (*) If NODE is a direct child of a shadow host but is not distributed into the flat tree, NODE@BeforeAnchor and NODE@AfterAnchor are valid Position but invalid PositionInFlatTree. This patch handles these two kind of positions. BUG= 702756 , 709872 , 712984 TEST=FrameSelectionTest.SelectInvalidPositionInFlatTreeDoesntCrash Review-Url: https://codereview.chromium.org/2850443002 Cr-Commit-Position: refs/heads/master@{#467676} [modify] https://crrev.com/0cbb9fc2999813a837b8ee0483f77e3879cc25e0/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/0cbb9fc2999813a837b8ee0483f77e3879cc25e0/third_party/WebKit/Source/core/editing/Position.cpp [modify] https://crrev.com/0cbb9fc2999813a837b8ee0483f77e3879cc25e0/third_party/WebKit/Source/core/editing/SelectionEditor.cpp
,
Apr 27 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by xiaoche...@chromium.org
, Mar 17 2017The root cause is that our current implementation of |isUserSelectContain()| is fake and checks only tag name: // 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) { return isHTMLTextAreaElement(node) || isHTMLInputElement(node) || isHTMLSelectElement(node); } A (not ultimate fix) is to check whether the node is laid out or not. In this way, canonicalization no longer tries to convert SELECT@AfterAnchor into a parent-anchored position.