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

Issue 596494 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Chrome extension options UI - right click menu in the wrong place.

Reported by lbell...@gmail.com, Mar 21 2016

Issue description

Chrome Version       : 49.0.2623.87
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
URLs (if applicable) : N/A
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5: N/A
  Firefox 4.x: N/A
     IE 7/8/9: N/A

What steps will reproduce the problem?
1. Create a chrome extension with the new options page (declared in the manifest as "options_ui"). Here is manifest.json:

{
"name": "test",
"version": "1.0",
"manifest_version": 2,
"description": "check out options page bug",
"browser_action": {
    "default_popup": "popup.html"
},
"options_ui": {
    "page": "options.html"
}
}

2. Add a <select> element with <option> elements to the options page. Here is options.html:

<!DOCTYPE HTML>
<html lang="en">
<head>
    <title>test</title>
</head>
<body>
    <select>
        <option>one</option>
        <option>two</option>
        <option>three</option>
    </select>
</body>
</html>

3. Try right clicking anywhere on the options page.
4. Try clicking on the select menu. 

What is the expected result?
-The right click context menu should appear where you clicked.
-The options of the select menu should drop down directly under the <select> element.

What happens instead of that?
-The right click context menu appears in seemingly random locations around the screen.
-The dropdown menu appears in seemingly random locations, not under the <select> element.


Please provide any additional information below. Attach a screenshot if
possible.

The problem goes away if you click the maximize/restore down button, but returns when the options page is refreshed. Attached is a screenshot of the right click menu in the wrong place.

UserAgentString: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36



 
right_click.PNG
70.0 KB View Download

Comment 1 by lbell...@gmail.com, Mar 21 2016

Edit: the problem is temporarily resolved (until the options page is reloaded) if the browser is resized in any way.
Components: Blink>HTML>Menu Blink>UI
Labels: Needs-Feedback
lbellist@Could you please confirm whether we can close this issue as per comment #1?

Comment 3 by lbell...@gmail.com, Mar 22 2016

No. As I said in the comment, the issue seems to go away when you resize the browser. But as soon as you reload the option page the problem returns.
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 23 2016

Labels: -Needs-Feedback Needs-Review
Owner: ssamanoori@chromium.org
Status: Assigned (was: Unconfirmed)
Thank you for providing more feedback. Assigning to requester "ssamanoori@chromium.org" for another review.

For more details visit https://sites.google.com/a/chromium.org/dev/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by rob@robwu.nl, Mar 25 2016

Cc: fsam...@chromium.org wkorman@chromium.org
Components: Platform>Extensions
Labels: -Type-Bug -Pri-2 -Needs-Review M-49 Pri-1 Type-Bug-Regression
Owner: chrishtr@chromium.org
This is a regression, it used to work in Chrome 48.

a86973e6a92be4540c7d1229e7b3be0148bafd2f is the culprit. When I manually revert this commit, the bug disappears.

Comment 6 by tkent@chromium.org, Apr 1 2016

Components: -Blink>UI Blink
Remove non-official component, Blink>UI.

Comment 7 by tkent@chromium.org, Apr 4 2016

Components: -Blink>HTML>Menu -Blink Blink>Layout

Comment 8 by e...@chromium.org, Apr 5 2016

Components: -Blink>Layout -Platform>Extensions Blink>Paint
My guess is that the problem is removal of the extra loop calling widgetPositionsUpdated in FrameView. That change will adjust the order in which the widgets get the call. I would try re-instituting that before reverting the deletion of "reportGeometry" in WebPluginContainerImpl.

Re-assign to me if you're swamped.
Owner: schenney@chromium.org
The problem is actually the removal of reportGeometry from WebPluginContainerImpl::setFrameRect, which is probably the perf issue. It seems that if setFrameRect is called then frameRectsChanged should be called, so I'll try to see what code sets the frame rects.
In case anyone is googling for this, I just want to point out that this also affects the drop-down box of the select element. It can potentially appear on the wrong monitor in multi-monitor setups.
Project Member

Comment 13 by bugdroid1@chromium.org, May 19 2016

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

commit b999309cd9ace1b703df73ed50d1c0d9efde90fa
Author: schenney <schenney@chromium.org>
Date: Thu May 19 17:42:41 2016

Fix positioning of widgets

The BrowserPlugin updateGeometry code was only sending a UpdateGeometry message to its host if the rect was the _same_ as the previous rect. This required two calls to the method in order to get the message sent, and no doubt resulted in the message being sent every time the geometry was the same and never when it was different.

R=fsamuel@chromium.org, lazyboy@chromium.org
BUG= 555201 , 596494 

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

[modify] https://crrev.com/b999309cd9ace1b703df73ed50d1c0d9efde90fa/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/b999309cd9ace1b703df73ed50d1c0d9efde90fa/content/renderer/browser_plugin/browser_plugin.cc

Status: Fixed (was: Assigned)
Labels: Merge-Request-51
This made the m52 branch but I think it's broken in m51 and probably m50 too. No negative fallout has come from the patch apart from increasing test flakiness (no doubt exacerbating a pre-existing test timing problem).

Comment 16 by tin...@google.com, May 25 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.
Before we approve merge to M51, Could you please confirm whether this change is baked/verified in Canary and safe to merge?
Could you confirm what you mean by baked/verified on Canary? Do you mean "verified as fixed" or do you mean "verified as not causing further issues"?

I have verified that it is fixed in m53 Canary, and it landed before the m52 branch, so it should already be fixed in m52 (I'll verify that when I restart my browser). I verified that it is broken in m51 so the merge is necessary if we want to fix the bug.

I have not seen any bug reports related to the fix, but then it's not my normal area so I would only know if a bisect blamed me. Is there more I should do?
It means verified as fixed as no regression.
I think that this is verified as fixed causing no regressions. I know it's fixed, and I have seen no evidence at all of further regressions.
Labels: -Merge-Review-51 Merge-Approved-51
Ok, approving merge to M51 branch 2704 based on comment #20. Please merge ASAP as we're cutting M51 Desktop Stable RC today. Thank you.
Project Member

Comment 22 by bugdroid1@chromium.org, May 31 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/859ed4fe376118092b622b79c99650e13704a3a4

commit 859ed4fe376118092b622b79c99650e13704a3a4
Author: Stephen Chenney <schenney@chromium.org>
Date: Tue May 31 16:59:55 2016

Fix positioning of widgets

The BrowserPlugin updateGeometry code was only sending a UpdateGeometry message to its host if the rect was the _same_ as the previous rect. This required two calls to the method in order to get the message sent, and no doubt resulted in the message being sent every time the geometry was the same and never when it was different.

R=fsamuel@chromium.org, lazyboy@chromium.org
BUG= 555201 , 596494 

Review-Url: https://codereview.chromium.org/1958903005
Cr-Commit-Position: refs/heads/master@{#394808}
(cherry picked from commit b999309cd9ace1b703df73ed50d1c0d9efde90fa)

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

Cr-Commit-Position: refs/branch-heads/2704@{#683}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/859ed4fe376118092b622b79c99650e13704a3a4/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/859ed4fe376118092b622b79c99650e13704a3a4/content/renderer/browser_plugin/browser_plugin.cc

Labels: TE-Verified-M51 TE-Verified-51.0.2704.78
Tested the issue on Windows 7 using 51.0.2704.78 as per steps in comment #0.Observed that the right click context menu appeared where we clicked and 
the options of the select menu drop down directly under the <select> element.
Please find attached screencast.

Marking it as TE-Verified.
596494.mp4
822 KB Download
Labels: TE-Verified-51.0.2704.79
Tested the issue on Windows 7 using 51.0.2704.79 as per steps in comment #0 and it is working fine.

Sign in to add a comment