Closed Bug 1795804 Opened 2 years ago Closed 2 years ago

Delay when maximizing browser window

Categories

(Firefox :: Tabbed Browser, defect)

Firefox 108
defect

Tracking

()

VERIFIED FIXED
108 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox106 --- wontfix
firefox107 --- wontfix
firefox108 --- verified

People

(Reporter: blakewolf, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(8 files)

Attached video 2022-10-17 20-39-44.mkv

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

Steps to reproduce:

When double clicking the title bar, there is a slight delay before the tabs move to the far left

Mozregression points to 1789823

Regressed by: 1789823

The Bugbug bot thinks this bug should belong to the 'Firefox::Tabbed Browser' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Tabbed Browser

:emilio, since you are the author of the regressor, bug 1789823, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

That's a bit surprising, specially because only https://hg.mozilla.org/integration/autoland/rev/c9785cd100fc is potentially relevant to windows...

Drive-by, but this timer is main-thread only, I have no idea why it'd
need a lock.

Assignee: nobody → emilio
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(emilio)

Also drive-by, but it's faster.

Depends on D159576

This patch doesn't change behavior by itself, but it makes the
persistent attribute code a bit clearer by making it use typed enums
rather than raw integers for flags. Also centralizes the update.

Depends on D159577

Much like everything else. This is important because the front-end uses
sizemode for styling.

The comment I'm removing doesn't really make much sense, since sizemode
and size are persisted separately (using different dirty flags) so it's
not like we're saving duplicate work or something.

The delay this fixes existed already on Linux since ~forever at least,
haven't tested macOS.

Depends on D159578

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f9bf0d3022af
Don't use lock to set persistent attribute timer. r=smaug
https://hg.mozilla.org/integration/autoland/rev/9df50f3e64f5
Use atom version of attributes in persistence code when possible. r=smaug
https://hg.mozilla.org/integration/autoland/rev/643aaf8f2efd
Centralize persistent attribute update code. r=smaug
https://hg.mozilla.org/integration/autoland/rev/d2e0da08b38f
Persist sizemode synchronously. r=smaug

Set release status flags based on info from the regressing bug 1789823

Backed out for causing assertion failures in widget/cocoa/nsCocoaWindow.mm

Backout link: https://hg.mozilla.org/integration/autoland/rev/a84adce7fced223b632c6faebc84134472614a58

Push with failures

Failure log

INFO - GECKO(1871) | [Parent 1871, Main Thread] ###!!! ASSERTION: mBounds out of sync!: 'mWindow && mBounds == r', file /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaWindow.mm:1851
Flags: needinfo?(emilio)
Flags: needinfo?(emilio)

This fixes issues like the one that got me backed out, where we have a
dirty size request, and we want to synchronously persist sizemode.

With this patch, we'd persist sizemode sync, but size would still be
updated async, preserving the previous size behavior.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

There's still more patches to land, the bug isn't closed.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e8f88bcb7a4
Persist only what's requested sync. r=smaug
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/491ac50c7721
Persist sizemode synchronously. r=smaug

Backed out changeset 491ac50c7721 (Bug 1795804) for causing mochitest failures on test_maximized_persist.xhtml.
Backout link
Push with failures <--> c3
Failure Log

Flags: needinfo?(emilio)

This actually avoids the assertion that got me backed out, sigh.

Flags: needinfo?(emilio)
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

sigh

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d5887b709cf4
Move code around a little bit to avoid calling GetRestoredBounds when not using them. r=smaug
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47e9c3d582b3
Persist sizemode synchronously. r=smaug
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Regressions: 1797469
Flags: qe-verify+
Regressions: 1801032
Regressions: 1801031
Regressions: 1801023

Reproduced the initial issue here using old Nightly build 2022-10-17, I verified that using latest beta build Firefox 108.0b5 the tabs transition is now smooth on Windows and Ubuntu when having both a few tabs or loaded with bunch of tabs.

I noticed that on macOS when having enough tabs to trigger the overflow tabs arrows when having the browser not in full screen and switching to full screen there is still a small delay for the arrows to be dismissed from the tabs strip (the arrows must go away because there are not enough tabs to trigger them when in full screen but enough when out of full screen). I captured the delay in a video, it is a bit glitchy though, hope it captures the delay I see live.
Let me know if you want me to log a new bug on this or if this delay is acceptable.

Flags: needinfo?(emilio)

I don't think it's a huge issue but can you file a new bug about that in any case? I'm curious to know if it's a regression. Thanks!

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #26)

I don't think it's a huge issue but can you file a new bug about that in any case? I'm curious to know if it's a regression. Thanks!

I logged bug 1802165 for that issue (I've added you to CC on it), it is not a regression since even 72.0a1 had that particular behavior.

Will go ahead and close this bug as verified fixed based on the above.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1801459
Regressions: 1801460
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: