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

Issue 695800 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: In Report an Issue overlay, unwanted space is seen at bottom of context menu.

Reported by lpa...@etouch.net, Feb 24 2017

Issue description

Chrome Version: 58.0.3022.0 55a8e3f7414855960a20135217d32de930466f3d-refs/heads/master@{#452713} (32/64-bit)
OS: Windows (7,8,10), Linux (14.04 LTS)

What steps will reproduce the problem?
1) Launch chrome, go to NTP, press Alt+Shift+I to open 'Report an Issue' overlay.
2) In URL, right click on 'newtab' text and observe the bottom of context menu.

Unwanted space is seen at bottom of context menu.

No such unwanted space should be seen at bottom of context menu. 

This is a Regression issue broken in M-57, will soon update other info
Manual bisect:
Good build: 57.0.2977.0 
Bad build: 57.0.2978.0 

Note: Issue is not seen on Mac OS.
 
report_issue.png
58.7 KB View Download
Labels: hasbisect-per-revision ReleaseBlock-Stable
Owner: eugene...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good Build -- 57.0.2977.0 (revision : 442447)
Bad Build  -- 57.0.2978.0 (revision : 442756)

You are probably looking for a change made after 442573 (known good), but no later than 442574 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/1a5a88a44dcfc171d6fff65abe844c9eb631c253..25371c65e843d76f82b0a828a71859c3862b9b3e

@eugenebut -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Adding RB LAbel as this is a recent Regression. Please remove if not required.
Thank You.

Comment 2 by gov...@chromium.org, Feb 24 2017


URGENT - PTAL ASAP.

We're getting VERY close to M57 Stable promotion. And 
this issue is marked as M57 stable release blocker. Pls make sure to land the fix and get it merged into the release branch ASAP so it gets enough baking time in Beta (before Stable promotion).

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M58.

Thank you.
Cc: gov...@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
This can not be related to my changes, because I change only ios-specific code which does not affect Windows. This bug needs triage and I don't know who owns context menus on Windows.

Comment 4 by gov...@chromium.org, Feb 24 2017

Cc: pbomm...@chromium.org
Prudhvi, could you ptal and bisect please.
Components: -UI>Browser>ReportAnIssue Blink>Editing>Spellcheck
Owner: edwardjung@chromium.org
Status: Assigned (was: Untriaged)
Please find the bisect : 

You are probably looking for a change made after 442544 (known good), but no lat
er than 442545 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/d38026c5a4b7a69552a4d7f41c37fdb735f0e283..8bedca0b30c55f439af1f13f580dcfb352856d8b
There's a separator added at the end of the editable items block expecting more items (usually Inspect). In the feedback Chrome App, inspect isn't allowed leaving the trailing separator. I'll remove.


Screen Shot 2017-02-24 at 23.41.33.png
343 KB View Download
Pending CL review: 
https://codereview.chromium.org/2713333002/

Comment 8 Deleted

edwardjung@ Gentle Ping! Since this issue is marked as RB-Stable van we get any latest update on this issue?

Thanks!
Sorry for the delay. I'm committing the CL now.
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 28 2017

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

commit 0f26be86266baa146ff07b69aace687393d8fc36
Author: edwardjung <edwardjung@chromium.org>
Date: Tue Feb 28 10:29:16 2017

Remove redundant separator at end of the spelling correction context menu

In some editing cases (reporting a issue) inspect is not the last item.

BUG= 695800 

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

[modify] https://crrev.com/0f26be86266baa146ff07b69aace687393d8fc36/chrome/browser/renderer_context_menu/render_view_context_menu.cc

URGENT - PTAL ASAP.

A friendly reminder that M57 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch by Friday (03/03), 5:00 PM PST in order to make into the desktop Stable final build cut. Thank you!

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M58.

Thank you.


Labels: TE-Verified-M58 TE-Verified-58.0.3027.0
Rechecked this on chrome version 58.0.3027.0 on Windows 10 and Ubuntu 14.04. No extra space is displayed. Attached screenshot for the same. Fix is working as intended. Adding TE-Verified labels.

Thanks.!
Nospace.png
35.8 KB View Download
Labels: Merge-Request-57
Project Member

Comment 15 by sheriffbot@chromium.org, Mar 1 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Change is working fine in Canary per comment #13.
 edwardjung@, could you please confirm there are no other regression and change is safe to merge to M57? Thank you.
Yes, it's working correctly. Cherry pick for merge to 2987 here, waiting for LGTM from lazyboy@ as I don't have committer access:

https://codereview.chromium.org/2721343003/
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 1 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc2bb753a4f87323a7d6473a6421d16f2e3da769

commit dc2bb753a4f87323a7d6473a6421d16f2e3da769
Author: edwardjung <edwardjung@chromium.org>
Date: Wed Mar 01 20:49:53 2017

Remove redundant separator at end of the spelling correction context menu

In some editing cases (reporting a issue) inspect is not the last item.

Review URL: https://codereview.chromium.org/2713333002

BUG= 695800 
NOTRY=true
NOPRESUBMIT=true

(cherry picked from commit 0f26be86266baa146ff07b69aace687393d8fc36)

Review-Url: https://codereview.chromium.org/2721343003
Cr-Commit-Position: refs/branch-heads/2987@{#730}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/dc2bb753a4f87323a7d6473a6421d16f2e3da769/chrome/browser/renderer_context_menu/render_view_context_menu.cc

Status: Fixed (was: Assigned)
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 1 2017

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

commit 85108a060f1fa35ab6f0eb6f45827bc881937f66
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Wed Mar 01 22:54:19 2017

Revert "Remove redundant separator at end of the spelling correction context menu"

The merge was incorrect, breaking compile. Will re-merge soon.

This reverts commit dc2bb753a4f87323a7d6473a6421d16f2e3da769.

BUG= 697621 ,  695800 
TBR=lazyboy@chromium.org

Review-Url: https://codereview.chromium.org/2722143003 .
Cr-Commit-Position: refs/branch-heads/2987@{#733}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/85108a060f1fa35ab6f0eb6f45827bc881937f66/chrome/browser/renderer_context_menu/render_view_context_menu.cc

Labels: -merge-merged-2987 Merge-Request-57
Need to merge this again since the first merge ^^^ broke compile ( crbug.com/697621 )
Labels: -Merge-Request-57 Merge-Approved-57
Approving merge to M57 branch 2987. Please merge ASAP. Hope build compiles successfully after the merge.
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 1 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a20477e538717f9db7e6bec4efac84d408e4531

commit 4a20477e538717f9db7e6bec4efac84d408e4531
Author: Istiaque Ahmed <lazyboy@chromium.org>
Date: Wed Mar 01 23:14:33 2017

[Re-Merge 57] Remove redundant separator at end of the spelling correction context menu

In some editing cases (reporting a issue) inspect is not the last item.

BUG= 695800 

Review-Url: https://codereview.chromium.org/2713333002
Cr-Commit-Position: refs/heads/master@{#453565}
(cherry picked from commit 0f26be86266baa146ff07b69aace687393d8fc36)

Review-Url: https://codereview.chromium.org/2724973002 .
Cr-Commit-Position: refs/branch-heads/2987@{#734}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/4a20477e538717f9db7e6bec4efac84d408e4531/chrome/browser/renderer_context_menu/render_view_context_menu.cc

Cc: hdodda@chromium.org
Labels: TE-Verified-57.0.2987.96 TE-Verified-M57
Verified on Windows 7 , ubuntu 14.04 using chrome M57 #57.0.2987.96 and issue is fixed.

No extra space is seen on context menu in report issue page.

Attached screenshot for reference.

Adding TE-Verififed labels.

Thanks!
698500.png
221 KB View Download

Sign in to add a comment