Commit 299a4daf authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Chromium LUCI CQ

Make hit testing after ::after content works correctly

This patch changes |NGInlineCursor::PositionForPointInInlineBox()| to
ignore pseudo element and layout generated text, e.g. ellipsis, and
style generated text, "content" in ::before/::after, to handle clicking
right of content of ::after correctly.

Before this patch, |closest_child_before| (point) holds fragment item
for value of CSS "content" in |PositionForPointInInlineBox()|, then
|PositionForPointInInlineBox()| returns null position because style
generated text is not DOM position.

After this patch, |closest_child_because| (point) holds fragment item
for last layout object having non-null |GetNode()| before pseudo
element ::after.

Bug: 1164974
Change-Id: Iaba2f0807c345b13463b956e009b730449121096
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627242
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843942}
parent e3a46e71
......@@ -149,6 +149,15 @@ class LayoutViewHitTestTest : public testing::WithParamInterface<HitTestConfig>,
GetFrame().GetSettings()->SetEditingBehaviorType(
GetParam().editing_behavior);
}
PositionWithAffinity HitTest(const PhysicalOffset offset) {
const HitTestRequest hit_request(HitTestRequest::kActive);
const HitTestLocation hit_location(offset);
HitTestResult hit_result(hit_request, hit_location);
if (!GetLayoutView().HitTest(hit_location, hit_result))
return PositionWithAffinity();
return hit_result.GetPosition();
}
};
INSTANTIATE_TEST_SUITE_P(
......@@ -614,4 +623,53 @@ TEST_P(LayoutViewHitTestTest, HitTestVerticalRLRoot) {
result.GetPosition());
}
// http://crbug.com/1164974
TEST_P(LayoutViewHitTestTest, PseudoElementAfterBlock) {
LoadAhem();
InsertStyleElement(
"body { margin: 0px; font: 10px/15px Ahem; }"
"p::after { content: 'XY' }");
SetBodyInnerHTML("<div><p id=target>ab</p></div>");
const auto& text_ab = *To<Text>(GetElementById("target")->firstChild());
// In legacy layout, this position comes from |LayoutBlock::PositionBox()|
// for mac/unix, or |LayoutObject::FindPosition()| on android/windows.
const auto expected = PositionWithAffinity(
IsAndroidOrWindowsEditingBehavior() ? Position(text_ab, 2)
: Position(text_ab, 0),
LayoutNG() && IsAndroidOrWindowsEditingBehavior()
? TextAffinity::kUpstream
: TextAffinity::kDownstream);
EXPECT_EQ(expected, HitTest(PhysicalOffset(20, 5))) << "after ab";
EXPECT_EQ(expected, HitTest(PhysicalOffset(25, 5))) << "at X";
EXPECT_EQ(expected, HitTest(PhysicalOffset(35, 5))) << "at Y";
EXPECT_EQ(expected, HitTest(PhysicalOffset(40, 5))) << "after Y";
EXPECT_EQ(expected, HitTest(PhysicalOffset(50, 5))) << "after XY";
}
TEST_P(LayoutViewHitTestTest, PseudoElementAfterBlockWithMargin) {
LoadAhem();
InsertStyleElement(
"body { margin: 0px; font: 10px/15px Ahem; }"
"p::after { content: 'XY'; margin-left: 10px;}");
SetBodyInnerHTML("<div><p id=target>ab</p></div>");
const auto& text_ab = *To<Text>(GetElementById("target")->firstChild());
// In legacy layout, this position comes from |LayoutBlock::PositionBox()|
// for mac/unix, or |LayoutObject::FindPosition()| on android/windows.
const auto expected = PositionWithAffinity(
IsAndroidOrWindowsEditingBehavior() ? Position(text_ab, 2)
: Position(text_ab, 0),
LayoutNG() && IsAndroidOrWindowsEditingBehavior()
? TextAffinity::kUpstream
: TextAffinity::kDownstream);
EXPECT_EQ(expected, HitTest(PhysicalOffset(20, 5))) << "after ab";
EXPECT_EQ(expected, HitTest(PhysicalOffset(25, 5))) << "at margin-left";
EXPECT_EQ(expected, HitTest(PhysicalOffset(30, 5))) << "before X";
EXPECT_EQ(expected, HitTest(PhysicalOffset(35, 5))) << "at X";
EXPECT_EQ(expected, HitTest(PhysicalOffset(45, 5))) << "at Y";
EXPECT_EQ(expected, HitTest(PhysicalOffset(50, 5))) << "after Y";
EXPECT_EQ(expected, HitTest(PhysicalOffset(55, 5))) << "after XY";
}
} // namespace blink
......@@ -68,6 +68,31 @@ bool IsLastBRInPage(const LayoutObject& layout_object) {
return layout_object.IsBR() && !layout_object.NextInPreOrder();
}
bool ShouldIgnoreForPositionForPoint(const NGFragmentItem& item) {
switch (item.Type()) {
case NGFragmentItem::kBox:
if (item.BoxFragment()) {
// Skip pseudo element ::before/::after
// All/LayoutViewHitTestTest.PseudoElementAfter* needs this.
return !item.GetLayoutObject()->NonPseudoNode();
}
// Skip virtually "culled" inline box, e.g. <span>foo</span>
// "editing/selection/shift-click.html" reaches here.
DCHECK(item.GetLayoutObject()->IsLayoutInline()) << item;
return true;
case NGFragmentItem::kGeneratedText:
return true;
case NGFragmentItem::kText:
// Returns true when |item.GetLayoutObject().IsStyleGenerated()|.
// All/LayoutViewHitTestTest.PseudoElementAfter* needs this.
return item.IsGeneratedText();
case NGFragmentItem::kLine:
NOTREACHED();
break;
}
return false;
}
} // namespace
inline void NGInlineCursor::MoveToItem(const ItemsSpan::iterator& iter) {
......@@ -620,13 +645,8 @@ PositionWithAffinity NGInlineCursor::PositionForPointInInlineBox(
for (; descendants; descendants.MoveToNext()) {
const NGFragmentItem* child_item = descendants.CurrentItem();
DCHECK(child_item);
if (child_item->Type() == NGFragmentItem::kBox &&
!child_item->BoxFragment()) {
// Skip virtually "culled" inline box, e.g. <span>foo</span>
// "editing/selection/shift-click.html" reaches here.
DCHECK(child_item->GetLayoutObject()->IsLayoutInline()) << child_item;
if (ShouldIgnoreForPositionForPoint(*child_item))
continue;
}
const LayoutUnit child_inline_offset =
child_item->OffsetInContainerFragment()
.ConvertToLogical(writing_direction, container_size,
......
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