New issue
Advanced search Search tips

Issue 722234 link

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



Sign in to add a comment

Make Position::operator==() not to take null Position

Project Member Reported by yoichio@chromium.org, May 15 2017 
ComparePositions(null,0, null,0) crashes with DCHECK
 but Position() == Position().
If we have compare(), we think operator==() is defined using the compare().
That's not true in Position and its variant types(VisiblePositions).

Comment 1 by yoichio@chromium.org, Jul 20 2017 

Step 1. Replace existing == operator usage to new Position::EqualsDeprecated,
 which is same as ==.
Step 2. Change operator== definition following same assumption in operator<=
Step 3. Replace EqualsDeprecated with new operator== removing redundant assumption 
 check.

Comment 2 by yosin@chromium.org, Jul 20 2017

Owner: ----
Summary: Make Position::operator==() not to take null Position (was: Clear Position::compare() and compare operators consistency. )
EqualDeprecated() can be written as

template <typename Strategy>
bool PostionTempalte<Strategy>::EqualsDeprecated(const PositionTempalte<Strategy>& other) {
  if (IsNull())
   return other.IsNull();
  if (other.isNull())
   return false;
  return *this == other;
}

For Step 1 in #c1, we should #if out operator==() then use compilation error to
detect usage.

We should not forget to replace operator!=() too.

Comment 3 by yosin@chromium.org, Sep 19 2017 

See also http://crbug.com/719343 Should not use Position::operator==() and operator!=()
Project Member

Comment 4 by sheriffbot@chromium.org, Sep 19

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

Comment 5 by yoichio@chromium.org, Sep 20

Status: Available (was: Untriaged)
►

Sign in to add a comment