Commit 4476bd69 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[LayoutNG] Handle upstream-after-line-break input in caret resolution

When the input is upstream after a line break, LayoutNG caret resolution
algorithm should resolve at the downstream position instead. This patch
does that and fixes 4 crashes when bidi caret affinity is enabled.

Bug: 894651
Change-Id: I178f1c0b209b907bd6ccad88b3e82d2df60f2907
Reviewed-on: https://chromium-review.googlesource.com/c/1404565
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarYoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621990}
parent 2e23f463
...@@ -239,6 +239,42 @@ NGCaretPosition AdjustCaretPositionForBidiText( ...@@ -239,6 +239,42 @@ NGCaretPosition AdjustCaretPositionForBidiText(
return BidiAdjustment::AdjustForCaretPositionResolution(caret_position); return BidiAdjustment::AdjustForCaretPositionResolution(caret_position);
} }
bool IsUpstreamAfterLineBreak(const NGCaretPosition& caret_position) {
if (caret_position.position_type != NGCaretPositionType::kAtTextOffset)
return false;
DCHECK(caret_position.fragment);
DCHECK(caret_position.fragment->PhysicalFragment().IsText());
DCHECK(caret_position.text_offset.has_value());
const NGPhysicalTextFragment& text_fragment =
ToNGPhysicalTextFragment(caret_position.fragment->PhysicalFragment());
if (!text_fragment.IsLineBreak())
return false;
return caret_position.text_offset.value() == text_fragment.EndOffset();
}
NGCaretPosition BetterCandidateBetween(const NGCaretPosition& current,
const NGCaretPosition& other,
unsigned offset,
TextAffinity affinity) {
DCHECK(!other.IsNull());
if (current.IsNull())
return other;
// There shouldn't be too many cases where we have multiple candidates.
// Make sure all of them are captured and handled here.
// Only known case: either |current| or |other| is upstream after line break.
DCHECK_EQ(affinity, TextAffinity::kUpstream);
if (IsUpstreamAfterLineBreak(current)) {
DCHECK(!IsUpstreamAfterLineBreak(other));
return other;
}
DCHECK(IsUpstreamAfterLineBreak(other));
return current;
}
} // namespace } // namespace
// The main function for compute an NGCaretPosition. See the comments at the top // The main function for compute an NGCaretPosition. See the comments at the top
...@@ -265,10 +301,8 @@ NGCaretPosition ComputeNGCaretPosition(const LayoutBlockFlow& context, ...@@ -265,10 +301,8 @@ NGCaretPosition ComputeNGCaretPosition(const LayoutBlockFlow& context,
return AdjustCaretPositionForBidiText(resolution.caret_position); return AdjustCaretPositionForBidiText(resolution.caret_position);
DCHECK_EQ(ResolutionType::kFoundCandidate, resolution.type); DCHECK_EQ(ResolutionType::kFoundCandidate, resolution.type);
// TODO(xiaochengh): We are not sure if we can ever find multiple candidate = BetterCandidateBetween(candidate, resolution.caret_position,
// candidates. Handle it once reached. offset, affinity);
DCHECK(candidate.IsNull());
candidate = resolution.caret_position;
} }
return AdjustCaretPositionForBidiText(candidate); return AdjustCaretPositionForBidiText(candidate);
......
...@@ -288,11 +288,7 @@ crbug.com/591099 tables/mozilla/bugs/bug2973.html [ Pass ] ...@@ -288,11 +288,7 @@ crbug.com/591099 tables/mozilla/bugs/bug2973.html [ Pass ]
crbug.com/591099 virtual/android/rootscroller/set-root-scroller.html [ Pass ] crbug.com/591099 virtual/android/rootscroller/set-root-scroller.html [ Pass ]
crbug.com/591099 virtual/android/rootscroller/set-rootscroller-before-load.html [ Pass ] crbug.com/591099 virtual/android/rootscroller/set-rootscroller-before-load.html [ Pass ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/pasteboard/copy-paste-white-space.html [ Pass ] crbug.com/591099 virtual/bidi-caret-affinity/editing/pasteboard/copy-paste-white-space.html [ Pass ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/click-in-focusable-link-should-not-clear-selection.html [ Crash ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/doubleclick-whitespace-crash.html [ Crash ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/legal-positions.html [ Crash ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/paint-hyphen.html [ Pass ] crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/paint-hyphen.html [ Pass ]
crbug.com/591099 virtual/bidi-caret-affinity/editing/selection/word-granularity.html [ Crash ]
crbug.com/916511 virtual/composite-after-paint/paint/background/scrolling-background-with-negative-z-child.html [ Failure Crash ] crbug.com/916511 virtual/composite-after-paint/paint/background/scrolling-background-with-negative-z-child.html [ Failure Crash ]
crbug.com/591099 virtual/composite-after-paint/paint/invalidation/box/margin.html [ Failure Pass ] crbug.com/591099 virtual/composite-after-paint/paint/invalidation/box/margin.html [ Failure Pass ]
crbug.com/591099 virtual/display-lock/display-lock/lock-after-append/measure-forced-layout.html [ Failure ] crbug.com/591099 virtual/display-lock/display-lock/lock-after-append/measure-forced-layout.html [ Failure ]
......
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