1875831 - Tab hover card changes window order

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Julian Gaibler]

Attachment: demo.mp4

Hello!

STR:

1. In Nightly on macOS set browser.tabs.cardPreview.enabled to true
2. Open two windows and position them in a way that they overlap and the tab strip of the partially covered window is still visible
3. On the window that is covered by the other, hover over a tab until the tab preview appears.
   (the video makes it a lot clearer I think)

Expected result:
The tab preview appears, but the window is still covered by the other window.

Actual result:
The window where the tab preview shows up is being moved on top of all others.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

If you apply the patch in bug 1875143, is this still reproducible?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Just tested myself. Yes, it's still reproducible with 1875143 applied.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Bug 1875553 offers a potential solution, which is not to show the tabpreview for backgrounded windows.

Comment 4

[Julian Gaibler]

Yeah, I can still reproduce it with that patch applied :(
Sorry

Edit: oops, sorry did not see you already tested

Comment 5

[Paul Zühlcke [:pbz]]

For me the preview seems to steal focus from any other application window, not just Firefox windows.

Comment 7

[Dave Townsend [:mossop]]

(In reply to Paul Zühlcke [:pbz] from comment #5)

For me the preview seems to steal focus from any other application window, not just Firefox windows.

Yes me too, I'm probably going to disable the feature for the time being because its so annoying.

Comment 8

[:Gijs (he/him)]

Is S3 accurate if people end up using about:config to disable the feature? It feels like something that'd block shipping this.

Comment 9

[Dave Townsend [:mossop]]

Attachment: Bug 1875831: Don't use arrow positioning for the tab preview panel. r=#tabbrowser-reviewers!

The panel isn't an arrow panel and the standard popup positioning appears to
work fine. It also seems that something about the arrow positioning logic
causes background windows to raise.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5f3b98b5997
Don't use arrow positioning for the tab preview panel. r=tabbrowser-reviewers,dao

Comment 11

[pstanciu]

Backed out for causing mochitests failures in browser_tab_preview.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/tabs/browser_tab_preview.js | Test timed out -

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1887aa693fdc
Don't use arrow positioning for the tab preview panel. r=tabbrowser-reviewers,dao

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/1887aa693fdc

Comment 14

[Bogdan Maris, Desktop QA]

Attachment: Still an issue after initial fix

I used the build from treeherder that has the fix and I can see that its not fixed completely. If using the steps from comment 0 it reproduces instantly when hovering over one tab from background but if you use the interaction between Firefox and another program (Firefox in background and your focus in another program), one must hover over the tabs in Firefox a couple of times until the focus jumps to Firefox (this is kind of intermittently reproducible). I tried this with a trackpad and mouse on MacOS 13 and macOS 14.
Reopening the bug based on the above.

Comment 15

[Dave Townsend [:mossop]]

I've narrowed the problem down to this call to orderFront. It appears to be being called on the popup but it brings the main window to the front, presumably because it is a parent of the popup or something? I'm not really sure if the call is needed, commenting it out doesn't appear to break popups in any way that I can see.

Markus, do you know what this orderFront call is meant to be for or if we could maybe skip it in this case somehow?

Comment 16

[Markus Stange [:mstange]]

(In reply to Dave Townsend [:mossop] from comment #15)

I've narrowed the problem down to this call to orderFront. It appears to be being called on the popup but it brings the main window to the front, presumably because it is a parent of the popup or something?

That sounds likely - specifically the parent window part.

Which exact call to orderFront are you referring to?

Also, does this have to be a panel or could it be a position:fixed element inside the main browser window? In Chrome the tab tooltip never extends beyond the main window's bounds, so I don't think it would need to extend beyond the browser window in Firefox.

Comment 17

[Dave Townsend [:mossop]]

(In reply to Markus Stange [:mstange] from comment #16)

(In reply to Dave Townsend [:mossop] from comment #15)

I've narrowed the problem down to this call to orderFront. It appears to be being called on the popup but it brings the main window to the front, presumably because it is a parent of the popup or something?

That sounds likely - specifically the parent window part.

Which exact call to orderFront are you referring to?

Sorry, meant to link to that: https://searchfox.org/mozilla-central/source/widget/cocoa/nsCocoaWindow.mm#942

Also, does this have to be a panel or could it be a position:fixed element inside the main browser window? In Chrome the tab tooltip never extends beyond the main window's bounds, so I don't think it would need to extend beyond the browser window in Firefox.

I'm not sure.

Comment 18

[DJ (:dwalker)]

(In reply to Markus Stange [:mstange] from comment #16)

Also, does this have to be a panel or could it be a position:fixed element inside the main browser window? In Chrome the tab tooltip never extends beyond the main window's bounds, so I don't think it would need to extend beyond the browser window in Firefox.

There was some discussion in the revision comments that moved us away from that. In short there were some alignment and layering problems with the UI that are probably fixable, but using a panel made them irrelevant. Since this is a dialog-like element, I think it ideally it should remain as a panel.

Comment 19

[Markus Stange [:mstange]]

(In reply to Dave Townsend [:mossop] from comment #15)

I've narrowed the problem down to this call to orderFront. It appears to be being called on the popup but it brings the main window to the front, presumably because it is a parent of the popup or something? I'm not really sure if the call is needed, commenting it out doesn't appear to break popups in any way that I can see.

Sorry I haven't yet had a chance to debug this. But in theory I would expect that you need at least one call to orderFront so that the window becomes visible at all. It's possible that we can skip subsequent orderFront calls if we know that the window is already visible. If removing that call doesn't break anything, then the next step would be to find out which orderFront call makes the window visible to begin with, and then think about whether that's enough.

Comment 20

[Dave Townsend [:mossop]]

Attachment: Bug 1875831: Only use one method to order popup windows. r=mstange!

Currently for popup windows with a parent window call orderFront and
addChildWindow, both of which set the popups window level. It appears that
we only need one of these for the popup to appear correctly, using both for
some reason has the side effect of also raising the parent window above all
other application windows. This patch skips the orderFront call when we are
also going to call addChildWindow.

Also removed the now redundant comment about _setWindowNumber.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87de8d00268a
Only use one method to order popup windows. r=spohl

Comment 22

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/87de8d00268a

Comment 23

[Bogdan Maris, Desktop QA]

I am unable to reproduce this anymore with the latest Nightly 126.0a1 on my macOS 13 macbook, closing as verified fixed.