New issue
Advanced search Search tips

Issue 689392 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

ApplyBlockElementCommand::doApply() has redundant null selection check

Project Member Reported by yosin@chromium.org, Feb 7 2017

Issue description

ApplyBlockElementCommand::doApply, L78-
if (visibleEnd.deepEquivalent() != visibleStart.deepEquivalent() &&
      isStartOfParagraph(visibleEnd)) {
    const Position& newEnd =
        previousPositionOf(visibleEnd, CannotCrossEditingBoundary)
            .deepEquivalent();
    SelectionInDOMTree::Builder builder;
    builder.collapse(visibleStart.toPositionWithAffinity());
    if (newEnd.isNotNull())
      builder.extend(newEnd);
    builder.setIsDirectional(endingSelection().isDirectional());
    const SelectionInDOMTree& newSelection = builder.build();

// This is redundant, since |visilbeStart| is not null here.
    if (newSelection.isNone())
      return;
    setEndingSelection(newSelection);
 }
 
Project Member

Comment 1 by sheriffbot@chromium.org, Feb 12 2018

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. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

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

Comment 3 by bugdroid1@chromium.org, Mar 20 2018

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

commit c75064c05568d94869822773ce134c66153a04aa
Author: Zhuoyu Qian <zhuoyu.qian@samsung.com>
Date: Tue Mar 20 10:54:12 2018

Remove redundant null check in ApplyBlockElementCommand::DoApply()

Since ApplyBlockElementCommand::DoApply() has checked |visible_start|
is not null before, and |new_selection|'s base is |visible_start|, so
|new_selection| can not be an empty selection and the null check of
|new_selection| is redundant, we can remove it.

Bug: 689392

Signed-off-by: Zhuoyu Qian <zhuoyu.qian@samsung.com>
Change-Id: Ic278963f06b05b65a2d1a08532b9a2a78ef530d3
Reviewed-on: https://chromium-review.googlesource.com/967631
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544325}
[modify] https://crrev.com/c75064c05568d94869822773ce134c66153a04aa/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp

Sign in to add a comment