Closed Bug 1695472 Opened 3 years ago Closed 3 years ago

[Fission] fullscreen embedded video clips causes all tabs to lose the 'x' close button

Categories

(Firefox :: Tabbed Browser, defect)

defect

Tracking

()

RESOLVED FIXED
91 Branch
Fission Milestone M7a
Tracking Status
firefox91 --- fixed

People

(Reporter: robuhde, Assigned: jaws)

References

Details

Attachments

(2 files)

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

Steps to reproduce:

I can reproduce by following these steps:
Fullscreen any embedded Twitch clip and then leave fullscreen by any means.

I'm unaware of other websites that embed Twitch clips or videos, however on https://old.reddit.com/r/LivestreamFail/ they are commonly posted.

Actual results:

Every single tab except for the currently active tab loses its 'x' to be able to close them. So moving to a new tab shows the x for that tab but hides it on all others.

This is fixed by either opening a new tab or closing a current tab.

Expected results:

Exiting fullscreen from an embedded Twitch clip should not change whether tabs show their close button.

I've found more, embedded youtube videos will also cause this. I used: https://support.google.com/youtube/answer/171780?hl=en

Hi robuhde,

I tried to reproduce the steps on Firefox Nightly 88.0a1 (2021-03-01) (64-bit) but I wasn't able to see the 'x' of the tabs disappearing.
Could you be more specific? Which pages were opened at the time? Would you be able to attach a screen recording to this ticket showing the issue?

Also, please test if you are able to reproduce this in the latest Firefox Nightly, you can download it from here: https://nightly.mozilla.org/

Thanks in advance,
Virginia

Flags: needinfo?(robuhde)

I show the current version, I have no add-ons enabled, I am logged out of my mozilla account, and open both a youtube and twitch clip embedded video in a private session (this still occurs in all other scenarios for me). I will also try to wipe all caches/data and update if that changes anything.

Flags: needinfo?(robuhde)

I uninstalled, refreshed everything and it did not persist. The default installation with my synced data (bookmarks, addons, options) is not affected. I will be verifying every change I make and I will update this with anything useful.

I managed to cause it to occur again, but I've repeated the actions again and I am unable to recreate anymore nor do I know exactly what triggered it.

Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → INACTIVE

I found what causes it. With a default installation the only thing I changed was setting fission.autostart to true in about:config. Fullscreening embed videos - which I believe is from any website embedding any video - causes every tab close 'x' button to disappear.

Status: RESOLVED → UNCONFIRMED
Resolution: INACTIVE → ---

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

Component: Untriaged → Tabbed Browser
Blocks: 1609791
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Fullscreening embedded Twitch clips causes all tabs to lose the 'x' close button → Fullscreening embedded video clips causes all tabs to lose the 'x' close button
Summary: Fullscreening embedded video clips causes all tabs to lose the 'x' close button → With fission.autostart enabled, fullscreen embedded video clips causes all tabs to lose the 'x' close button
Blocks: fission-dogfooding
No longer blocks: 1609791
Summary: With fission.autostart enabled, fullscreen embedded video clips causes all tabs to lose the 'x' close button → [Fission] fullscreen embedded video clips causes all tabs to lose the 'x' close button
Version: Firefox 88 → unspecified
Fission Milestone: --- → M7a

This reproduces fairly easily for me. The effect here is the same as when too many tabs are open, and individual tab widths are too small, and close buttons on non-selected tabs are hidden. In fact, it appears to be caused by the same attribute. When this bug is occurring, the close button is hidden because the <tabs> has a closebuttons="activetab" attribute on it.

I alt-tabbed from the fullscreen view to the browser toolbox while the video was fullscreen, and discovered that the attribute isn't set until fullscreen is exited, so it appears that at some point while fullscreen is being exited, _updateCloseButtons() is being called and the browser is incorrectly deciding that the tabs are too small and the close buttons should be hidden, and it's not being fixed again at a later point. (https://searchfox.org/mozilla-central/rev/46a67b8656ac12b5c180e47bc4055f713d73983b/browser/base/content/tabbrowser-tabs.js#1165)

Unfortunately I don't know enough about how these events are fired to know why this would be happening off the top of my head.

Gijs, you might be familiar with this code. Can you help debug/fix this please?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Neha Kochar [:neha] from comment #9)

Gijs, you might be familiar with this code. Can you help debug/fix this please?

Passing this potato (unsure of temperature) to Jared, who's been working on tabs for MR1.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(jaws)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)

Neil, I see you reviewed the code at https://searchfox.org/mozilla-central/rev/1ba472c91bbde4b127f6e614ae72888ab59611cf/browser/base/content/browser-fullScreenAndPointerLock.js#456-459. What would you recommend we do here? Without knowing if the resize event is from a fullscreen change we can't always wait for the attribute to change before proceeding here.

Flags: needinfo?(enndeakin)

(In reply to (Away until May 24) Jared Wein [:jaws] (please needinfo? me) from comment #12)

Neil, I see you reviewed the code at https://searchfox.org/mozilla-central/rev/1ba472c91bbde4b127f6e614ae72888ab59611cf/browser/base/content/browser-fullScreenAndPointerLock.js#456-459. What would you recommend we do here? Without knowing if the resize event is from a fullscreen change we can't always wait for the attribute to change before proceeding here.

I didn't really review that part, but a simple test suggests that just calling _updateCloseButtons() when fullscreen is exited in cleanupDomFullscreen() seems to fix this issue.

Flags: needinfo?(enndeakin)
Attachment #9224544 - Attachment description: Bug 1695472 - WIP, increase the delay when updating tab close buttons to wait for the inDOMFullscreen attribute to get updated. → Bug 1695472 - Update the tab close buttons after exiting DOM fullscreen since the calculating the width during exit/resize is prone to errors with 0-width tabs.
Attachment #9224544 - Attachment description: Bug 1695472 - Update the tab close buttons after exiting DOM fullscreen since the calculating the width during exit/resize is prone to errors with 0-width tabs. → Bug 1695472 - Use a ResizeObserver to update the tab close button states, improving performance through coalesence of resize events and increased delay of responding to give more time for Fission communication.
Attachment #9224544 - Attachment description: Bug 1695472 - Use a ResizeObserver to update the tab close button states, improving performance through coalesence of resize events and increased delay of responding to give more time for Fission communication. → Bug 1695472 - Use a ResizeObserver and MutationObserver to update the tab close button states, improving performance through coalesence of events.
Attachment #9224544 - Attachment description: Bug 1695472 - Use a ResizeObserver and MutationObserver to update the tab close button states, improving performance through coalesence of events. → Bug 1695472 - Add a mutation observer for fullscreen changes so we don't have to rely on timing of resize events.
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fa771d9f62d
Add a mutation observer for fullscreen changes so we don't have to rely on timing of resize events. r=Gijs
Regressions: 1714691
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

Backed out changeset 7fa771d9f62d (Bug 1695472) for causing bc failures in browser_bug1620341.js (Bug 1714691)
Backout link: https://hg.mozilla.org/integration/autoland/rev/304616299acb63a861b1c68b041c67d564dbc25a
Push with failures, failure log.

Status: RESOLVED → REOPENED
Flags: needinfo?(jaws)
Resolution: FIXED → ---
Target Milestone: 91 Branch → ---
No longer regressions: 1714691
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ad71dde9ed5e
Add a mutation observer for fullscreen changes so we don't have to rely on timing of resize events. r=Gijs
Regressions: 1715208
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Flags: needinfo?(jaws)
Blocks: 1794449
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: