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

Issue 675514 link

Starred by 1 user
Status: Fixed
Owner:
ekaramad@chromium.org
Closed: Jan 2017
Cc:
█ hdodda@chromium.org
nyerramilli@chromium.org
█ ranjitkan@chromium.org
mac-bugs-priority@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unnecessary Services option is seen in context menu.

Reported by vvishwak...@etouch.net, Dec 19 2016 
Chrome Version: 57.0.2955.0 (Official Build) 111fdd030ce49ce1de525ac10a983bed8fd8720e-refs/heads/master@{#439360} (64-bit)
OS: Mac (10.11.6, 10.12.1)

What steps will reproduce the problem?
1) Launch chrome, go to ‘chrome://version’ and press Cmd+A.
2) Now click anywhere on the page and then reload the page.
3) Right click anywhere on the page and observe.

Unnecessary ‘Services’ option is seen in context menu.

Unnecessary ‘Services’ option should not be seen in context menu.

This is a Regression issue broken in M-54 series, will soon update other info
Manual bisect
Good build: 54.0.2832.0 (412743)
Bad build: 54.0.2833.0 (413134)

Note: Issue is not seen on Windows and Linux OS.
services_actual.mov
2.5 MB Download
services_expected.mov
2.2 MB Download

Comment 1 by hdodda@chromium.org, Dec 19 2016

Cc: hdodda@chromium.org
Labels: hasbisect-per-revision
Owner: ekaramad@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 54.0.2832.0 (412743)
Bad build: 54.0.2833.0 (413134)

You are probably looking for a change made after 412937 (known good), but no later than 412938 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

  https://chromium.googlesource.com/chromium/src/+log/8368a085571762ca8fc5002e5827063d142c9674..65cf5593f03c43ccaa8f97b023909040934d9ff5

From the CL above, assigning the issue to the concern owner 

@ekaramad - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Review-Url: https://codereview.chromium.org/2240553003

Thanks!

Comment 2 by ekaramad@chromium.org, Dec 19 2016

Status: Started (was: Assigned)
Thanks! I will take a look.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 23 2016 

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

commit a7d3c829829627d5739ee07b24c79ea97fc57886
Author: ekaramad <ekaramad@chromium.org>
Date: Fri Dec 23 00:08:51 2016

Fix an issue with not clearing |selected_text_| when text selection range is empty (Mac)

After the CL (https://codereview.chromium.org/2240553003) on text selection
tracking (for OOPIF) the new implementation missed some minor details in
tracking text selection. Specificaly, the value of RWHVMac::selected_text_
is not cleared when reported text or range from renderer is empty.

This has caused a regression in context menu items where some of the items shown
could relate to the previous selection.

This CL will:
    1- Make sure |selected_text_| is cleared when reported text selection from
       renderer is empty.

    2- Makes sure the overal behavior is perserved from that of text selection
       before OOPIF-IME changes.

BUG= 675514 

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

[modify] https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/a7d3c829829627d5739ee07b24c79ea97fc57886/content/browser/renderer_host/text_input_manager.cc

Comment 4 by ekaramad@chromium.org, Jan 3 2017

Status: Fixed (was: Started)
Marking as fixed since this issue is no longer reproduced on Canary. We might still be able to merge this to M56 is necessary.
►

Sign in to add a comment