Skip to content

GitLab

T
tangled
Commit a1eabca2 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

Handle atomic inlines correctly in NG PositionForPoint(). 

When an atomic inline is the closest element, but we're not *inside* it,
we shouldn't look for stuff inside it, either.

(Note that the change here in LayoutBlock::PositionForPoint() will make
us enter PositionForPointIfOutsideAtomicInlineLevel() also for (inline)
tables, since the atomic inline check has been moved before the table
check. This doesn't make much of a difference, though, since
LayoutBox::PositionForPoint() already has special code for tables, which
does pretty much exactly the same as
PositionForPointIfOutsideAtomicInlineLevel()).

The test included here used to fail with LayoutNGFullPositionForPoint
enabled, in the inline-flex case, but also in the inline-table case, if
LayoutNGTable is enabled in addition.

This also fixes
GranularityStrategyTest.UpdateExtentWithNullPositionForCharacter and
GranularityStrategyTest.UpdateExtentWithNullPositionForDirectional on
Android and Windows (when LayoutNGFullPositionForPoint is enabled).

Bug: 1150362
Change-Id: Ied71317aaaa8e380600461436ee979a56b5a4584
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552866
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831026}
parent b86b8cd6
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 7 deletions
third_party/blink/renderer/core/layout/layout_block.cc
View file @ a1eabca2
...@@ -1416,6 +1416,13 @@ PositionWithAffinity LayoutBlock::PositionForPoint( ...@@ -1416,6 +1416,13 @@ PositionWithAffinity LayoutBlock::PositionForPoint(
const PhysicalOffset& point) const { const PhysicalOffset& point) const {
NOT_DESTROYED(); NOT_DESTROYED();
if (IsAtomicInlineLevel()) {
PositionWithAffinity position =
PositionForPointIfOutsideAtomicInlineLevel(point);
if (!position.IsNull())
return position;
}
if (IsLayoutNGObject() && PhysicalFragmentCount() && if (IsLayoutNGObject() && PhysicalFragmentCount() &&
RuntimeEnabledFeatures::LayoutNGFullPositionForPointEnabled()) { RuntimeEnabledFeatures::LayoutNGFullPositionForPointEnabled()) {
// Layout engine boundary. Enter NG PositionForPoint(). Assert // Layout engine boundary. Enter NG PositionForPoint(). Assert
...@@ -1427,13 +1434,6 @@ PositionWithAffinity LayoutBlock::PositionForPoint( ...@@ -1427,13 +1434,6 @@ PositionWithAffinity LayoutBlock::PositionForPoint(
if (IsTable()) if (IsTable())
return LayoutBox::PositionForPoint(point); return LayoutBox::PositionForPoint(point);
if (IsAtomicInlineLevel()) {
PositionWithAffinity position =
PositionForPointIfOutsideAtomicInlineLevel(point);
if (!position.IsNull())
return position;
}
PhysicalOffset point_in_contents = point; PhysicalOffset point_in_contents = point;
OffsetForContents(point_in_contents); OffsetForContents(point_in_contents);
LayoutPoint point_in_logical_contents = FlipForWritingMode(point_in_contents); LayoutPoint point_in_logical_contents = FlipForWritingMode(point_in_contents);
......
third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc
View file @ a1eabca2
...@@ -808,6 +808,14 @@ PositionWithAffinity NGPhysicalBoxFragment::PositionForPoint( ...@@ -808,6 +808,14 @@ PositionWithAffinity NGPhysicalBoxFragment::PositionForPoint(
return layout_object_->PositionForPoint(point); return layout_object_->PositionForPoint(point);
} }
// TODO(layout-dev): Handle situations where we're near (but not within)
// atomic inlines here, rather than relying on it being taken care of by the
// layout object. This is currently handled in LayoutBlock and
// LayoutNGBlockFlowMixin - look for
// PositionForPointIfOutsideAtomicInlineLevel().
DCHECK(!IsAtomicInline() ||
PhysicalRect(PhysicalOffset(), Size()).Contains(point));
if (IsScrollContainer()) if (IsScrollContainer())
point += PhysicalOffset(PixelSnappedScrolledContentOffset()); point += PhysicalOffset(PixelSnappedScrolledContentOffset());
......
third_party/blink/web_tests/fast/dom/Document/CaretRangeFromPoint/atomic-inlines.html 0 → 100644
View file @ a1eabca2
<!DOCTYPE html>
<style>
body { margin:0; }
.container { margin:30px; line-height:20px; }
#inlineblock { display: inline-block; }
#inlinetable { display: inline-table; }
#inlineflex { display: inline-flex; }
#inlinegrid { display: inline-grid; }
</style>
<div class="container">
<div id="inlineblock"><div>asdfgh</div></div>
</div>
<div class="container">
<div id="inlinetable"><div>asdfgh</div></div>
</div>
<div class="container">
<div id="inlineflex"><div>asdfgh</div></div>
</div>
<div class="container">
<div id="inlinegrid"><div>asdfgh</div></div>
</div>
<script src="../../../../resources/testharness.js"></script>
<script src="../../../../resources/testharnessreport.js"></script>
<script>
function test_outside(y, expected_node) {
range = document.caretRangeFromPoint(0, y);
assert_equals(range.startContainer, expected_node);
assert_equals(range.startOffset, 0);
range = document.caretRangeFromPoint(300, y);
assert_equals(range.startContainer, expected_node);
assert_equals(range.startOffset, 1);
}
test(()=> { test_outside(40, inlineblock); }, "inline-block");
test(()=> { test_outside(90, inlinetable); }, "inline-table");
test(()=> { test_outside(140, inlineflex); }, "inline-flex");
test(()=> { test_outside(190, inlinegrid); }, "inline-grid");
</script>
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