1680537 - Tab close button visibility code doesn't seem to deal with the case where "overflow" event is fired after resize, causing tab close buttons to be hidden when they shouldn't be

Status: NEW

(Firefox :: Tabbed Browser, defect)

Description

[Saverio Morelli]

Attachment: -1-.png

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

Steps to reproduce:

Version: 84.0b7 (64-bit), OS: Windows 10 Pro

When you have many tabs opened, the you resize the window (until all tabs aren't visible anymore), the re-maximise the window: now the "close" buttons are hidden (you need to open a new tab to fix it).

I've attached screenshots

Actual results:

The close buttons aren't visible

Expected results:

The close buttons should be visible again

Comment 1

[Saverio Morelli]

Attachment: -2-.png

Comment 2

[Saverio Morelli]

Attachment: -3-.png

Comment 3

[BugBot [:suhaib / :marco]]

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

Comment 4

[Alice0775 White]

I can reproduce the issue Firefox 75 as well as Nightly85.0a1 Windows10.

Reproducible : Almost 100% reproducible under the following STR

My test environment: Monitor 1920*1080, Windows10 1909

STR for bisect:

1. Start with new profile
2. Enable titlebar
3. Open 8 tabs
4. Reduce browser width so that tabbar will overflow(i.e <> buttons appear)
5. Maximized

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e84157c7ee828bbca6c62ad836aaca28161dab95&tochange=95fe1470b3718d04e5c45605faedda62da140071

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Nika, is this something we could fix for 93/94?
What should be the priority in this case?
Thank you!

Comment 6

[Nika Layzell [:nika] (ni? for response)]

I see that :dao moved this into the XPCOM component, but I don't see how the behaviour of the tab close icons is related to XPCOM specifically, given that it's controlled by the tabbed browser frontend. ni? :dao to potentially move back to Tabbed Browser.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

I think Dao moved it to match where the regression is coming from - bug 1615607

Needinfo on smaug as he is the author of the regressor

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Farre, can you have a look? Looks like the assumptions made in bug 1615607 weren't entirely correct?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

This bug hints that some frontend code has wrong assumptions about scheduling.
(but this could be also something else having such assumptions)
Unfortunately I can't reproduce this, tested on linux and Windows.

Comment 10

[Alice0775 White]

FWIW,
When problem happens
Evaluating gBrowser.tabContainer._updateCloseButtons(); then the close buttons is displayed again.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Ok, so something is not calling that or something related when needed.
Or do we possibly have some issue with ResizeObserver

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

I think the frontend code looks reasonable.
I believe https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/browser/base/content/tabbrowser-tabs.js#88-89,92 should get called

dholbert, any ideas why ResizeObserver wouldn't be triggered

Comment 13

[Emilio Cobos Álvarez [:emilio]]

They're observing the documentElement size, does using window.addEventListener("resize") work out of curiosity? In this case it should be ~equivalent.

Comment 14

[Alice0775 White]

FWIW,
after evaluating the following code, it does not work.
window.addEventListener("resize",()=>{gBrowser.tabContainer._updateCloseButtons();})

after evaluating the following code, it works as expected.
window.addEventListener("resize",()=>{setTimeout(() => {gBrowser.tabContainer._updateCloseButtons();}, 0);})

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Oh, I wonder if it's interacting badly with the overflow/underflow events.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Ah, yes, there is probably some race condition between https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/browser/base/content/tabbrowser-tabs.js#1176-1179 and resize notifications.

https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/layout/generic/nsGfxScrollFrame.cpp#5958 is triggered after https://searchfox.org/mozilla-central/rev/2e3b0483e31abffe0b4374480a34c6d23f5186ea/dom/base/ResizeObserverController.cpp#21,23

AddWillPaintObserver behaves in a bit odd way (see bug 1488953, which would remove it), but still, there might be ordering issue in resize vs. some other state.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Back to frontend. It doesn't seem to deal with the case where "overflow" event is fired after resize, yet that can happen, and that behavior has been there for ages.