1626741 - Address bar panel/view/popup opens behind other popups/menus, and focus is also broken

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[leifeld]

Attachment: addressbar not in foreground

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Open download menue
"strg + l" to focus address bar (which opens in background??)
type something and suggestion appears
press arrow or tab to use suggestion

Actual results:

address bar dissapears

Expected results:

1. the address should be in front of download menue
2. tab or arrow select suggestion

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Drew Willcoxon :adw]

This is due to the fact that the urlbar view isn't a popup but the downloads panel is, like other panels in the toolbar. The worst part is that arrow key focus remains in the panel despite it looking like it's in the urlbar.

We should probably close all popups when the urlbar view opens.

Comment 3

[Dão Gottwald [:dao]]

My naive approach would be to monitor popup events on the whole window and then close any open popups when opening the address bar results. Is there a better way?

Comment 4

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Comment 8

[Neil Deakin]

(In reply to Dão Gottwald [::dao] from comment #3)

My naive approach would be to monitor popup events on the whole window and then close any open popups when opening the address bar results. Is there a better way?

Probably need to find a way to get the address bar popup to implement nsIRollupListener so it can be treated like a popup that can open and rollup in coordination with other popups.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:adw, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 10

[Marco Bonardo [:mak]]

(In reply to Neil Deakin from comment #8)

(In reply to Dão Gottwald [::dao] from comment #3)

My naive approach would be to monitor popup events on the whole window and then close any open popups when opening the address bar results. Is there a better way?

Probably need to find a way to get the address bar popup to implement nsIRollupListener so it can be treated like a popup that can open and rollup in coordination with other popups.

Sorry for pinging after so much time, but to make the bug actionable, we must better understand how that could be done.
Should we basically add something to the popup manager where we can register new nsIRollupListener instances that are not popups, or there could be a better way of doing that? Should we file a dependency bug in the popup component to investigate that?

Comment 11

[Neil Deakin]

Sorry for pinging after so much time, but to make the bug actionable, we must better understand how that could be done.
Should we basically add something to the popup manager where we can register new nsIRollupListener instances that are not popups

If you wanted a more detailed implementation that was able to able to handle the address bar dropdown as if it was a popup, it would likely involve something like that.

For this specific bug though, you could also just add a way to call nsPopupManager::Rollup() which will hide any open popups that aren't tooltips and have noautohide set. That would handle when the address bar dropdown is called, but not when some another popup is opened afterwards.

Comment 12

[Marco Bonardo [:mak]]

Off-hand, I think one could add a rollupAllPopups() method to nsIAppWindow, in the implementation just

  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
    pm->Rollup({});
  }

appWindow is reachable through window.docShell.treeOwner.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIAppWindow)

I don't know if this is the right approach, but it may be worth trying (I'll try it locally).

Comment 13

[Marco Bonardo [:mak]]

Attachment: Bug 1626741 - Roll-up other panels when the Address Bar results panel opens. r=NeilDeakin!,jteow!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a2ec4353ec1
Roll-up other panels when the Address Bar results panel opens. r=NeilDeakin,jteow

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4a2ec4353ec1

Comment 16

[Iulian Moraru]

Backed out for causing bc failures on browser_fullscreen_context_menu.js.

• Push with failures

• Failure log

• Backout link

[task 2024-05-25T04:03:40.561Z] 04:03:40     INFO - TEST-PASS | browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js | Expected no errors verifying context menu items - 
[task 2024-05-25T04:03:40.561Z] 04:03:40     INFO - waiting for fullscreen
[task 2024-05-25T04:03:40.563Z] 04:03:40     INFO - trigger the context menu
[task 2024-05-25T04:03:40.567Z] 04:03:40     INFO - context menu should be open, verify its menu items
[task 2024-05-25T04:03:40.568Z] 04:03:40     INFO - Got actual context menu items: 
[task 2024-05-25T04:03:40.568Z] 04:03:40     INFO - 
[task 2024-05-25T04:03:40.568Z] 04:03:40     INFO - Buffered messages finished
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js | Context menu has the expected number of items - Got +0, expected 10
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - Stack trace:
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/browser-test.js:test_is:1620
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochitests/content/browser/browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js:verifyContextMenu:124
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochitests/content/browser/browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js:testContextMenu/<:91
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - resource://testing-common/BrowserTestUtils.sys.mjs:withNewTab:146
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochitests/content/browser/browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js:testContextMenu:24
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/browser-test.js:handleTask:1139
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1211
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1353
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1128
[task 2024-05-25T04:03:40.575Z] 04:03:40     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058
[task 2024-05-25T04:03:40.576Z] 04:03:40     INFO - TEST-PASS | browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js | Expected no errors verifying context menu items - 
[task 2024-05-25T04:03:40.753Z] 04:03:40     INFO - Leaving test bound testContextMenu
[task 2024-05-25T04:03:41.167Z] 04:03:41     INFO - GECKO(1680) | MEMORY STAT | vsize 11826MB | residentFast 501MB | heapAllocated 253MB
[task 2024-05-25T04:03:41.168Z] 04:03:41     INFO - TEST-OK | browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js | took 1500ms
[task 2024-05-25T04:03:41.181Z] 04:03:41     INFO - checking window state
[task 2024-05-25T04:03:41.190Z] 04:03:41     INFO - TEST-START | browser/base/content/test/fullscreen/browser_fullscreen_cross_origin.js

Comment 17

[Iulian Moraru]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/bc46b838c113

Comment 18

[Marco Bonardo [:mak]]

I suspect it's the call to Browser:OpenLocation the test does to ensure the toolbox is visible, without following it with a gURLBar.blur(). If the urlbar panel opens delayed it will close the context menu that the test is using.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7206820516c1
Roll-up other panels when the Address Bar results panel opens. r=NeilDeakin,jteow

Comment 20

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/7206820516c1