New issue
Advanced search Search tips

Issue 719343 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Should not use Position::operator==() and operator!=()

Project Member Reported by yosin@chromium.org, May 8 2017

Issue description

We should introduce Position::IsEquivalent() as replacement of usages of
operator==() and operator!=() to handle Position(node, 0) and Position::FirstPositionInNode(node) as same position.
 

Comment 1 by yosin@chromium.org, May 8 2017

IsEquivalent(const Position& a, const Position& b) can be implemented as
a.ToOffsetInAnchor() == b.toOffsetInAnchor()
but this is slow if offset is large.

Project Member

Comment 2 by bugdroid1@chromium.org, May 15 2017

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

commit 4aee85c54527cbd0857b07d6dc4699465f55a18f
Author: yosin <yosin@chromium.org>
Date: Mon May 15 07:35:00 2017

Introduce Position::IsEquivalent()

This patch introduces |Position::IsEquivalent()| to check whether two positions
are in same position regardless anchor type, e.g. |Position(node, 0)| and
|Position::FirstPositionInNode(node)|. denote a position before first child of
|node|.

This patch is a preparation of patch[1].

[1] http://crrev.com/2875933004: Utilize Position::IsEquivalent() in
{Up,Down}streamIgnoringEditingBoundaries()

BUG=716093, 719343
TEST=run_webkit_unit_tests --gtest_filter=PositionTest.IsEquivalent

Review-Url: https://codereview.chromium.org/2875253002
Cr-Commit-Position: refs/heads/master@{#471689}

[modify] https://crrev.com/4aee85c54527cbd0857b07d6dc4699465f55a18f/third_party/WebKit/Source/core/editing/Position.cpp
[modify] https://crrev.com/4aee85c54527cbd0857b07d6dc4699465f55a18f/third_party/WebKit/Source/core/editing/Position.h
[modify] https://crrev.com/4aee85c54527cbd0857b07d6dc4699465f55a18f/third_party/WebKit/Source/core/editing/PositionTest.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, May 19 2017

Labels: merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b67267c9b096b7ac8fd187bccd91178c04689e32

commit b67267c9b096b7ac8fd187bccd91178c04689e32
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Fri May 19 10:29:37 2017

Introduce Position::IsEquivalent()

This patch introduces |Position::IsEquivalent()| to check whether two positions
are in same position regardless anchor type, e.g. |Position(node, 0)| and
|Position::FirstPositionInNode(node)|. denote a position before first child of
|node|.

This patch is a preparation of patch[1].

[1] http://crrev.com/2875933004: Utilize Position::IsEquivalent() in
{Up,Down}streamIgnoringEditingBoundaries()

BUG=716093, 719343
TEST=run_webkit_unit_tests --gtest_filter=PositionTest.IsEquivalent

Review-Url: https://codereview.chromium.org/2875253002
Cr-Original-Commit-Position: refs/heads/master@{#471689}
Review-Url: https://codereview.chromium.org/2890403002 .
Cr-Commit-Position: refs/branch-heads/3071@{#628}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/b67267c9b096b7ac8fd187bccd91178c04689e32/third_party/WebKit/Source/core/editing/Position.cpp
[modify] https://crrev.com/b67267c9b096b7ac8fd187bccd91178c04689e32/third_party/WebKit/Source/core/editing/Position.h
[modify] https://crrev.com/b67267c9b096b7ac8fd187bccd91178c04689e32/third_party/WebKit/Source/core/editing/PositionTest.cpp

Labels: Type-Task
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 14

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)

Sign in to add a comment