Skip to content

Commit

Permalink
Remove resolved TODO comment
Browse files Browse the repository at this point in the history 
It seems the job in TODO comment has been resolved.
So I think we can safely remove it.

Change-Id: Ic9d4b919e72373382f5da8828e5d890e9494b009
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2297071
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Hyunjune Kim <hyunjune.kim@samsung.com>
Cr-Commit-Position: refs/heads/master@{#789836}
  • Loading branch information
@pitachips
pitachips authored and Commit Bot committed Jul 20, 2020
1 parent 5c31d54 commit a946c51
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 71 deletions.
1 change: 1 addition & 0 deletions AUTHORS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ HyeockJin Kim <kherootz@gmail.com>
Hyungchan Kim <inlinechan@gmail.com>
Hyungwook Lee <hyungwook.lee@navercorp.com>
Hyungwook Lee <withlhw@gmail.com>
HyunJi Kim <hjkim3323@gmail.com>
Hyunjun Shin <hyunjun.shin2@navercorp.com>
Hyunjune Kim <hyunjune.kim@samsung.com>
Hyunki Baik <hyunki.baik@samsung.com>
Expand Down
13 changes: 3 additions & 10 deletions third_party/blink/renderer/core/editing/visible_units.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class IntPoint;
class IntRect;
class LocalFrame;

// |WordSide| is used as a parameter of |StartOfWord()| and |EndOfWord()|
// to control a returning position when they are called for a position before
// word boundary.
// |WordSide| is used as a parameter of |StartOfWordPosition()| and
// |EndOfWord()| to control a returning position when they are called for a
// position before word boundary.
enum WordSide {
kNextWordIfOnBoundary = false,
kPreviousWordIfOnBoundary = true
Expand Down Expand Up @@ -114,18 +114,11 @@ PreviousPositionOf(const VisiblePositionInFlatTree&,
EditingBoundaryCrossingRule = kCanCrossEditingBoundary);

// words
// TODO(yoichio): Replace |startOfWord| to |startOfWordPosition| because
// returned Position should be canonicalized with |previousBoundary()| by
// TextItetator.
CORE_EXPORT Position StartOfWordPosition(const Position&,
WordSide = kNextWordIfOnBoundary);
CORE_EXPORT VisiblePosition StartOfWord(const VisiblePosition&,
WordSide = kNextWordIfOnBoundary);
CORE_EXPORT PositionInFlatTree
StartOfWordPosition(const PositionInFlatTree&,
WordSide = kNextWordIfOnBoundary);
CORE_EXPORT VisiblePositionInFlatTree
StartOfWord(const VisiblePositionInFlatTree&, WordSide = kNextWordIfOnBoundary);
CORE_EXPORT VisiblePosition EndOfWord(const VisiblePosition&,
WordSide = kNextWordIfOnBoundary);
CORE_EXPORT Position EndOfWordPosition(const Position&,
Expand Down
11 changes: 0 additions & 11 deletions third_party/blink/renderer/core/editing/visible_units_word.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,15 +339,4 @@ Position StartOfWordPosition(const Position& position, WordSide side) {
StartOfWordPosition(ToPositionInFlatTree(position), side));
}

VisiblePosition StartOfWord(const VisiblePosition& position, WordSide side) {
return CreateVisiblePosition(
StartOfWordPosition(position.DeepEquivalent(), side));
}

VisiblePositionInFlatTree StartOfWord(const VisiblePositionInFlatTree& position,
WordSide side) {
return CreateVisiblePosition(
StartOfWordPosition(position.DeepEquivalent(), side));
}

} // namespace blink
123 changes: 73 additions & 50 deletions third_party/blink/renderer/core/editing/visible_units_word_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ class VisibleUnitsWordTest : public EditingTestBase {
const std::string& selection_text,
WordSide word_side = WordSide::kNextWordIfOnBoundary) {
const Position position = SetSelectionTextToBody(selection_text).Base();
return GetCaretTextFromBody(
StartOfWord(CreateVisiblePosition(position), word_side)
.DeepEquivalent());
return GetCaretTextFromBody(StartOfWordPosition(position, word_side));
}

std::string DoEndOfWord(
Expand Down Expand Up @@ -194,60 +192,85 @@ TEST_P(ParameterizedVisibleUnitsWordTest, StartOfWordShadowDOM) {
Node* five = shadow_root->getElementById("five")->firstChild();
Node* space = shadow_root->getElementById("space")->firstChild();

EXPECT_EQ(
Position(one, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*one, 0)).DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(space, 1),
StartOfWord(CreateVisiblePositionInFlatTree(*one, 0)).DeepEquivalent());

EXPECT_EQ(
Position(one, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*one, 1)).DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(space, 1),
StartOfWord(CreateVisiblePositionInFlatTree(*one, 1)).DeepEquivalent());

EXPECT_EQ(
Position(one, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*two, 0)).DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(four, 0),
StartOfWord(CreateVisiblePositionInFlatTree(*two, 0)).DeepEquivalent());

EXPECT_EQ(
Position(four, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*two, 1)).DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(four, 0),
StartOfWord(CreateVisiblePositionInFlatTree(*two, 1)).DeepEquivalent());

EXPECT_EQ(Position(one, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*one, 0).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(PositionInFlatTree(space, 1),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*one, 0).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(Position(one, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*one, 1).DeepEquivalent()))
.DeepEquivalent());

EXPECT_EQ(PositionInFlatTree(space, 1),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*one, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(Position(one, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*two, 0).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(PositionInFlatTree(four, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*two, 0).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(Position(four, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*two, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(PositionInFlatTree(four, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*two, 1).DeepEquivalent()))
.DeepEquivalent());
// DOM tree canonicalization moves the result to a wrong position.
EXPECT_EQ(
Position(two, 2),
StartOfWord(CreateVisiblePositionInDOMTree(*three, 1)).DeepEquivalent());
EXPECT_EQ(Position(two, 2),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*three, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(three, 0),
StartOfWord(CreateVisiblePositionInFlatTree(*three, 1)).DeepEquivalent());

EXPECT_EQ(
Position(four, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*four, 1)).DeepEquivalent());
EXPECT_EQ(
PositionInFlatTree(four, 0),
StartOfWord(CreateVisiblePositionInFlatTree(*four, 1)).DeepEquivalent());

EXPECT_EQ(
Position(one, 0),
StartOfWord(CreateVisiblePositionInDOMTree(*five, 1)).DeepEquivalent());
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*three, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(Position(four, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*four, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(PositionInFlatTree(four, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*four, 1).DeepEquivalent()))
.DeepEquivalent());
EXPECT_EQ(Position(one, 0),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInDOMTree(*five, 1).DeepEquivalent()))
.DeepEquivalent());
// Flat tree canonicalization moves result to downstream position
EXPECT_EQ(
PositionInFlatTree(space, 1),
StartOfWord(CreateVisiblePositionInFlatTree(*five, 1)).DeepEquivalent());
EXPECT_EQ(PositionInFlatTree(space, 1),
CreateVisiblePosition(
StartOfWordPosition(
CreateVisiblePositionInFlatTree(*five, 1).DeepEquivalent()))
.DeepEquivalent());
}

TEST_P(ParameterizedVisibleUnitsWordTest, StartOfWordTextSecurity) {
// Note: |StartOfWord()| considers security characters as a sequence "x".
// Note: |StartOfWordPosition()| considers security characters
// as a sequence "x".
InsertStyleElement("s {-webkit-text-security:disc;}");
EXPECT_EQ("|abc<s>foo bar</s>baz", DoStartOfWord("|abc<s>foo bar</s>baz"));
EXPECT_EQ("|abc<s>foo bar</s>baz", DoStartOfWord("abc|<s>foo bar</s>baz"));
Expand Down

0 comments on commit a946c51

Please sign in to comment.