Closed Bug 1707379 Opened 1 year ago Closed 5 months ago

Firefox raises window when it gains focus when using yabai with focus_follows_mouse with autofocus, but not autoraise

Categories

(Core :: Widget: Cocoa, defect, P5)

Firefox 90
defect

Tracking

()

RESOLVED FIXED
97 Branch
Tracking Status
firefox97 --- fixed

People

(Reporter: 6mjjmugn96, Assigned: bnhunsaker)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

On MacOS (10.13.6):

  • install yabai (brew install koekeishiya/formulae/yabai)
  • configure it to do focus_follows_mouse with autofocus, but not autoraise (echo 'yabai -m config focus_follows_mouse autofocus' > ~/.yabairc)
  • run yabai (brew services start yabai)
  • open Firefox
  • open any other apps
  • place other apps' windows above the Firefox window
  • hover mouse pointer over Firefox window

Actual results:

  • Firefox window focuses and becomes the topmost window

Expected results:

  • Firefox window should focus and remain in its existing Z position

No other application I've found raises itself the way Firefox does. Firefox has done this for ages. This is not a new bug.

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Cocoa' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Widget: Cocoa
Product: Firefox → Core
Severity: -- → S4
Priority: -- → P5
Summary: Firefox raises window when it gains focus → Firefox raises window when it gains focus when using yabai with focus_follows_mouse with autofocus, but not autoraise

Most other applications focus but retain Z-order. When Firefox gains focus, it jumps to the front, in front of other windows. This causes slow downs in workflow as the Firefox window must be resized to reveal the window below.
Desired behaviour:
Firefox gains focus but does not change Z-order.
There is a bug submitted against the yabai project although it is proving difficult to debug since only some applications exhibit the misbehaviour
https://github.com/koekeishiya/yabai/issues/397

I understand that this is an esoteric combination of applications but I want to stress how essential both yabai and Firefox are to my workflow, I bet there are others in the same situation.

I am observing similar behavior in X11 since Firefox 90, which may be related. To reproduce:

  • Run an X11 window manager
    • Focus follows mouse configuration (sometimes known as "mouse focus" or "sloppy focus")
    • Autoraise disabled
  • Launch Firefox and leave running for some time, or enable an addon, which causes the bug to manifest immediately
  • Move the cursor into a Firefox window that is partially obscured by another window. The Firefox window will be raised to the top.
  • Once it begins, autoraising behavior persists until Firefox is restarted.

This occurred in some past versions, but I have not seen it for many releases. I do not remember which specific past versions exhibited the behavior, but my recollection is that it would occasionally appear in a new Firefox release, only to be fixed again (intentionally or not). It has now returned in Firefox 90 and 91.

In the X11 case, unlike what is described for yabai, Firefox does not immediately begin autoraising, but only begins to do so after it has been running for a period of time. Enabling an addon, however, will immediately cause this behavior, which persists until Firefox is restarted. Past versions of Firefox that had this problem would also begin autoraising when a new Private Window was created, but creating Private Windows in Firefox 90 and 91 does not have this effect.

This patch seems to fix the problem for me:

--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ -944,7 +944,9 @@ void nsCocoaWindow::Show(bool bState) {
       if (mAlwaysOnTop) {
         [mWindow orderFront:nil];
       } else {
-        [mWindow makeKeyAndOrderFront:nil];
+        if ( ! [mWindow isVisible] ) { // The fix for Bug 1707379
+          [mWindow makeKeyAndOrderFront:nil];
+        }
       }
       NS_OBJC_END_TRY_IGNORE_BLOCK;
       SendSetZLevelEvent();
Assignee: nobody → bnhunsaker

We have these lines of code at the beginning of that method:

  // We need to re-execute sometimes in order to bring already-visible
  // windows forward.
  if (!mSheetNeedsShow && !bState && ![mWindow isVisible]) return;

So it seems like this behavior is at least somewhat intentional (though maybe misguided), so we need to make sure that we're not unintentionally breaking any behavior that we'd like to keep.
Looking at the blame of those lines of code, it looks like the "bring to front on focus" behavior was implemented 14 years ago in bug 419338. The steps to reproduce in that bug refer to the download manager window, which no longer exists in Firefox. However, the bookmarks library window still exists. If you have that window open, then focus a main browser window, and then press Cmd+Shift+O, does Firefox still bring the library window to the front in builds with this patch?

Status: UNCONFIRMED → NEW
Ever confirmed: true

I tested my patch by opening the bookmarks library window, I then focused the main browser window. When I pressed Cmd_Shift+O The Library window came to the front as expected. So that still works as expected.

I built the current nightly mozilla-unified (97.0a1 (2021-12-17) (64-bit)) with the patch suggested and it resolves my problem. I did not find any issues with external windows: About or Library. Cmd+Shift+O opens the Library window or brings it to front if it is already open. Thank you so much for this, I hope it makes it into the release soon.

Thanks for testing! I'm going to propose a slightly different patch once I've run it through the test suite.

Here's a build with the alternative patch: target.dmg
Could you test this build, too?

The test runs look good: https://treeherder.mozilla.org/jobs?repo=try&revision=302af024a8fd673f152e5e8d17e66ce5ddc43afe

This primarily affects alternative window managers which allow focusing background windows.

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

Here's a build with the alternative patch: target.dmg
Could you test this build, too?

The test runs look good: https://treeherder.mozilla.org/jobs?repo=try&revision=302af024a8fd673f152e5e8d17e66ce5ddc43afe

If one of you could please confirm Markus' fix it would be much appreciated. Thanks!

Flags: needinfo?(les.m.grieve)
Flags: needinfo?(bnhunsaker)
Flags: needinfo?(6mjjmugn96)

I've tested Markus' fix, and it works for me without any issues. resolving this bug. Hopefully this can get pushed to the general release soon.

Flags: needinfo?(bnhunsaker)

I have also been using this target.dmg 97.0a1 (2021-12-27) for a few hours and it works splendid. I did not notice a difference between the two patches. Thank you! Thank you!

Flags: needinfo?(les.m.grieve)
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/419a3314370e
Don't re-show visible windows, in order to avoid unintended window ordering changes on focus. r=spohl

Thanks everyone for testing! I've landed the patch, it should become available in Nightly tomorrow.

Flags: needinfo?(6mjjmugn96)

Thank you!

Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch

(In reply to Sandor Molnar from comment #19)

https://hg.mozilla.org/mozilla-central/rev/419a3314370e

This patch appears to only affect the macOS version, but this problem manifests in X11, too. Many X11 window managers treat window focus and window ordering separately, and Firefox breaks these by raising windows to the top of the stack when they are focused.

You need to log in before you can comment on or make changes to this bug.