New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 715059 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 715889



Sign in to add a comment

FOCUS: Wrong context menu after hiding selection

Reported by dchau...@etouch.net, Apr 25 2017

Issue description

Chrome Version: 60.0.3080.0 (Official Build)70d2a00a72c50c344436f139b2a3f5c09ec337bf-refs/heads/master@{#466837} 32/64-bit.
OS: Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.10.5, 10.11.4).

What steps will reproduce the problem?
1. Launch chrome and navigate to chrome://md-settings/ page.
2. Keep focus on 'Learn more' link (or any link) under 'People' section using 'Tab' key.
3. Now, press 'Shift' + 'F10' key from keyboard to open context menu and observe.

Text box context menu is displayed for 'Learn more' link.
Link context menu should display for 'Learn more' link.

This is a regression  issue, broken in M-60 series, below is manual regression range.

Good build: 59.0.3071.0
Bad build: 60.0.3072.0

Kindly review the attached screen-cast for reference.
 
Actual behavior.mp4
429 KB View Download
Expected behavior.mp4
584 KB View Download
Labels: hasbisect-per-revision Proj-MaterialDesign-WebUI
Owner: hu...@opera.com
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 59.0.3071.0 (Revision: 464641).
Bad build : 60.0.3072.0 (Revision: 464836).

You are probably looking for a change made after 464697 (known good), but no later than 464698 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/188c1056bbfb9c98eca691c12f5641d4204f6066..0f65d25a4097959d977dbb1077717323438240a6

@hugoh: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You.

Comment 2 by hu...@opera.com, Apr 25 2017

Cc: yosin@chromium.org dpa...@chromium.org
Owner: dpa...@chromium.org
Status: Available (was: Assigned)
I cannot explain this one. dpapad@, can you? 

@yosin, let's revert my CL until we understand these regressions better? 

Comment 3 by dpa...@chromium.org, Apr 25 2017

Cc: dbeam@chromium.org

Comment 4 by dpa...@chromium.org, Apr 25 2017

Owner: yosin@chromium.org
Status: Assigned (was: Available)
Assigning to yosin@ for reverting. I tried to revert but the original CL can't be rolled back cleanly at this point.

Comment 5 by hu...@opera.com, Apr 26 2017

Components: -UI>Settings Blink>Editing>Selection
Summary: Wrong context menu after hiding selection (was: Regression: [MD Settings] wrong context menu is displayed for any link on Settings page.)
When the selection is hidden, context menu should use focused element as context (not the caret position). 

Minimal repro:

<input autofocus><a href="www"">link</a>
1. Hit tab key to go focus the link.
2. Shift+F10.
Expected: Context menu for <a>.
Actual: Context menu for <input>.


Comment 6 by hu...@opera.com, Apr 26 2017

Owner: hu...@opera.com
Status: Started (was: Assigned)
Here's a quick fix: https://codereview.chromium.org/2841093002

Comment 7 by yosin@chromium.org, Apr 27 2017

Blocking: 715889

Comment 8 by yosin@chromium.org, Apr 27 2017

Cc: hu...@opera.com
Owner: dchau...@etouch.net
Status: Assigned (was: Started)
Summary: FOCUS: NOT-REPRO: Wrong context menu after hiding selection (was: Wrong context menu after hiding selection)
I could not reproduce on 60.0.3081.0.
Context menu is showed at link, e.g. "learn more"

Could you do bisect? I would like to know which CL fixed this issue.

Comment 9 by yosin@chromium.org, Apr 27 2017

Owner: hu...@opera.com
Status: Started (was: Assigned)
Summary: FOCUS: Wrong context menu after hiding selection (was: FOCUS: NOT-REPRO: Wrong context menu after hiding selection)
Please ignore #c8. I could reproduce with 60.0.3081.0
Sorry for confusion.
 Issue 715905  has been merged into this issue.
Project Member

Comment 11 by bugdroid1@chromium.org, May 11 2017

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

commit 8549ed4eaf9d95db494bd7028bf17110b26b6918
Author: hugoh <hugoh@opera.com>
Date: Thu May 11 05:28:29 2017

Algorithm for deciding if a frame's selection should be hidden

Background:
Crrev.com/464698 introduced "hiding" of unfocused selections
in text controls. Hiding avoids clearing the selection upon change
of focus.

Problem:
Now we only hide selections inside text controls.
Selections within content-editable elements must also be hidden.

Solution:
Generalize previous work into an algorithm that, given current
DOM and its activeElement, determines whether a frame's selection
should be hidden.

See the algorithm in InHidden() for documentation and read its
unit tests in FrameSelectionTest.cpp.

BUG= 715059 ,  715889 

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

[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/8549ed4eaf9d95db494bd7028bf17110b26b6918/third_party/WebKit/Source/core/editing/LayoutSelection.h

Project Member

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

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

commit ff7930cd4d17f38e69016d96024acadf8a008a4d
Author: hugoh <hugoh@opera.com>
Date: Mon May 15 10:28:59 2017

Make context menu aware of hidden selection

When the frame's selection is hidden, the context menu should
use the focused element (not the selection) as context.

BUG= 715059 

Expected: Context menu for <a>.
Review-Url: https://codereview.chromium.org/2869713003
Cr-Commit-Position: refs/heads/master@{#471719}

[modify] https://crrev.com/ff7930cd4d17f38e69016d96024acadf8a008a4d/third_party/WebKit/LayoutTests/NeverFixTests
[add] https://crrev.com/ff7930cd4d17f38e69016d96024acadf8a008a4d/third_party/WebKit/LayoutTests/fast/events/contextmenu-follows-focus.html
[modify] https://crrev.com/ff7930cd4d17f38e69016d96024acadf8a008a4d/third_party/WebKit/Source/core/input/EventHandler.cpp

Comment 13 by hu...@opera.com, May 15 2017

Status: Fixed (was: Started)

Sign in to add a comment