New issue
Advanced search Search tips

Issue 828719 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Unify PlainTextRange and TextOffsetMapping

Project Member Reported by xiaoche...@chromium.org, Apr 4 2018 
The old class PlainTextRange and the newly introduced class TextOffsetMapping pretty much implements the same thing: wrap a TextIterator for conversion between plain text offsets and DOM positions.

Currently, they are wrapping TextIterator in their own way. For code health, we should unify their implementation.

We can start with simplifying PlainTextRange by making it wrap CharacterIterator, and then investigate the possibility of reimplementing TextOffsetMapping on top of PlainTextRange.

Comment 1 by yosin@chromium.org, Apr 5 2018

Labels: -Pri-2 Hotlist-CodeHealth Hotlist-GoodFirstBug Pri-3
Owner: ----
Status: Available (was: Assigned)
For first step, |PlainTextRange| should be marked final class.
Then, make |PlainTextRange| to hold scope element to make stand alone with
removing scope parameter from |CreateRange()|.

See the one pager of TextOffsetMapping[1] for details.

[1] https://goo.gl/v2Ax8d
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 6 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6e9060e2fc9da498346064e79860666f9db9177

commit b6e9060e2fc9da498346064e79860666f9db9177
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Apr 06 02:53:13 2018

Unify start and end calculation in PlainTextRange::CreateRangeFor()

This patch unifies the duplicate code in the function for better code
health.

Bug: 828719
Change-Id: I5c75a6345b345633bebd37c556b780cfaad24d1d
Reviewed-on: https://chromium-review.googlesource.com/996569
Reviewed-by: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548648}
[modify] https://crrev.com/b6e9060e2fc9da498346064e79860666f9db9177/third_party/WebKit/Source/core/editing/PlainTextRange.cpp
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 9 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6b2f2fc782c30fb429b48aa81d57f641822e80b

commit e6b2f2fc782c30fb429b48aa81d57f641822e80b
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Mon Apr 09 21:28:15 2018

Simplify handling of out-of-bound end offset in PlainTextRange::CreateRangeFor()

This patch refactors the function, so that it uses distinct return paths
for in-bound and out-of-bound end offsets, so that the code flow logic
is cleaner.

Bug: 828719
Change-Id: I5c4553b4e5480a2fcf0b2496e0153f648c002bb3
Reviewed-on: https://chromium-review.googlesource.com/999160
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549283}
[modify] https://crrev.com/e6b2f2fc782c30fb429b48aa81d57f641822e80b/third_party/blink/renderer/core/editing/plain_text_range.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b26e4da7f69910f713803e0647fdc8c9ecab252e

commit b26e4da7f69910f713803e0647fdc8c9ecab252e
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Tue Apr 10 18:23:02 2018

Reduce ToOffsetInAnchor() calls in PlainTextRange::CreateRangeFor()

This patch reduces the ToOffsetInAnchor() conversions to make the code
look cleaner.

Bug: 828719
Change-Id: Iadcb0a7ff40dc77b3f8edbc24744e27e702cbb83
Reviewed-on: https://chromium-review.googlesource.com/1004008
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549596}
[modify] https://crrev.com/b26e4da7f69910f713803e0647fdc8c9ecab252e/third_party/blink/renderer/core/editing/plain_text_range.cc
►

Sign in to add a comment