Commit be54a823 authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Fix text box offset passing in LayoutText::PositionForPoint

CreatePositionWithAffinityForBoxAfterAdjustingOffsetForBiDi() expects
an |offset| in range from 0 to box length. However, a call site in
LayoutText::PositionForPoint() incorrectly adds |box->Start()| to it,
causing out of range offsets.

This patch fixes it. Haven't found any incorrect behavior in wild due
to this bug, but it's an obvious bug so let's still fix it...

Change-Id: Id7deb528b18aa4c36d0a18bb564ede3dcc138110
Reviewed-on: https://chromium-review.googlesource.com/c/1289684
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600954}
parent 24a053ba
......@@ -751,10 +751,7 @@ CreatePositionWithAffinityForBoxAfterAdjustingOffsetForBiDi(
ShouldAffinityBeDownstream should_affinity_be_downstream) {
DCHECK(box);
DCHECK_GE(offset, 0);
// TODO(layout-dev): Stop passing out-of-range |offset|.
if (static_cast<unsigned>(offset) > box->Len())
offset = box->Len();
DCHECK_LE(static_cast<unsigned>(offset), box->Len());
if (offset && static_cast<unsigned>(offset) < box->Len()) {
return CreatePositionWithAffinityForBox(box, box->Start() + offset,
......@@ -822,8 +819,7 @@ PositionWithAffinity LayoutText::PositionForPoint(
return CreatePositionWithAffinityForBoxAfterAdjustingOffsetForBiDi(
last_box,
last_box->OffsetForPosition(point_line_direction, IncludePartialGlyphs,
BreakGlyphs) +
last_box->Start(),
BreakGlyphs),
should_affinity_be_downstream);
}
return CreatePositionWithAffinity(0);
......
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