Closed Bug 1588386 Opened 5 years ago Closed 5 years ago

Firefox enters a strange state without its UI elements after current tab being closed by extension when in fullscreen

Categories

(Firefox :: General, defect, P1)

Product:
Firefox
Component:
General
Version:
71 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 72
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 blocking verified
firefox72 + verified

People

(Reporter: lilydjwg, Assigned: ablayelyfondou)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [rca - Unhandled exceptions])

Attachments

(1 file)

Bug 1588386 - Restore toolbars when fullscreen mode exits due to a tab close.
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta+
Details | Review

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

  1. install vimium-ff (amo: https://addons.mozilla.org/firefox/addon/vimium-ff/ source: https://github.com/philc/vimium)
  2. open YouTube to watch a video
  3. go to fullscreen
  4. press 'x' to close current tab

Actual results:

The browser is in a strange state that the tabbar, urlbar and bookmark bar is not visible / usable. Switching to next / prev tabs using Ctrl + PgDn / PgUp does work.

Expected results:

When a fullscreen tab is closed by extension, Firefox should exit fullscreen correctly.

I get a 404 error when running mozregression:

mozregression --good 2019-10-10 --bad 2019-10-11 --profile-persistence=reuse

6:54.93 INFO: Narrowed inbound regression window from [a7ca5ad3, c5e6477c] (3 builds) to [6660bc0d, c5e6477c] (2 builds) (~1 steps left)
6:54.93 INFO: No more inbound revisions, bisection finished.
6:54.93 INFO: Last good revision: 6660bc0d1b23ae45dc3c952b6e21c0335e9739e9
6:54.93 INFO: First bad revision: c5e6477c3a245a97d4c3cdd5d3e406f8abaf94ad
6:54.93 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6660bc0d1b23ae45dc3c952b6e21c0335e9739e9&tochange=c5e6477c3a245a97d4c3cdd5d3e406f8abaf94ad

6:58.13 ERROR: The url u'https://hg.mozilla.org/integration/mozilla-inbound/json-pushes?changeset=c5e6477c3a245a97d4c3cdd5d3e406f8abaf94ad&full=1' returned a 404 error. Please check the validity of the url.

This happens on another video site too. Closing using Ctrl-w does work as expected.

BTW, to recover from this state, one can open the page again (by ctrl-shift-t or using the command line and navigate to some video page), enter fullscreen mode and then exit (by pressing esc), and then things are back to normal.

Status: UNCONFIRMED → NEW
Has STR: --- → yes
status-firefox70: --- → unaffected
status-firefox71: --- → affected
Ever confirmed: true
OS: Unspecified → All
Hardware: Unspecified → All

go to fullscreen
press 'x' to close current tab

added: the video player to fullscreen. press 'x' key.

I get a 404 error when running mozregression:

I got the same result.

suspects: bug 1505916

Has Regression Range: --- → yes
Regressed by: 1505916
Assignee: nobody → ablayelyfondou

Thanks YF, I will have a look at this.

More minimal STR (not requiring an add-on):

  1. Visit any YouTube video
  2. Fullscreen the video
  3. Hit Ctrl-W to close the tab

ER:

Fullscreen exits as the tab closes.

AR:

Fullscreen exits, but the browser still has hidden chrome, so the toolbars are all missing.

Thanks Mike! Your STR might not work on Linux though as mentioned in comment 1.

Component: Untriaged → XUL Widgets
Product: Firefox → Toolkit
Priority: -- → P1
Component: XUL Widgets → General
Product: Toolkit → Firefox

See phabricator comments. (I don't know who sees and what notifications from phabricator, so using needinfo)

Flags: needinfo?(ablayelyfondou)
See Also: → 1590138
Flags: needinfo?(ablayelyfondou)

Any updates on this bug, Abdoulaye? Do you think you'll have time to fix it, or should we hand it off? The pressure is on a little bit because we're already almost a week into Beta. :/

Flags: needinfo?(ablayelyfondou)

Hey Mike! Sorry, I've been pretty packed lastly but now I have more time to work on it. I am sure you will see some updates very soon.

Flags: needinfo?(ablayelyfondou)

Hey abdoulaye - just tracking the progress here - is the patch at the state where you need re-review, or are you working on a new revision?

Flags: needinfo?(ablayelyfondou)

Yeah, It just need a re-review here from smaug specially, I am not sure though if he got my new comment. So I will NI him here.

Flags: needinfo?(ablayelyfondou) → needinfo?(bugs)

Given the impact and the number of duplicates that show how easy to get into this state, we can't ship 71 with this bug so I am marking it as a blocker. We will need an uplift to beta and manual QA on the fix.

status-firefox72: --- → affected
tracking-firefox71: + → blocking
tracking-firefox72: --- → +
Severity: normal → major

This can land now.

(In reply to Abdoulaye O. LY from comment #19)

This can land now.

The new way to request checkin needed is to add the tag Check-in needed to the phabricator revision.

(In reply to Mark Banner (:standard8) from comment #20)

(In reply to Abdoulaye O. LY from comment #19)

This can land now.

The new way to request checkin needed is to add the tag Check-in needed to the phabricator revision.

Thanks Mark!

Flags: needinfo?(bugs)
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efe8b524f8bf
Restore toolbars when fullscreen mode exits due to a tab close. r=smaug

Abdoulaye, could you make an uplift request to beta please? I'd like to get this patch in our next beta if possible. Thanks!

Flags: needinfo?(ablayelyfondou)

https://hg.mozilla.org/mozilla-central/rev/efe8b524f8bf

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox72: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Comment on attachment 9101444 [details]
Bug 1588386 - Restore toolbars when fullscreen mode exits due to a tab close.

Beta/Release Uplift Approval Request

  • User impact if declined: Users can get into a state where their browser UI is missing if the tab in which they're watching fullscreen content closes while still in fullscreen.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 4.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a reasonably small, well contained patch that just adds an additional error handling case.
  • String changes made/needed:
Attachment #9101444 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I tested this issue on Ubuntu 16.04, Windows 10 and Mac OS X 10.14 with FF Nightly 72 (2019-11-04) and I can't reproduce the issue. However I also tested a FF Nightly 71 in order to try to reproduce the issue, on the same OSes, but I wasn't able to do it, I used both scenarios from comment 0 and comment 4.
lilydjwg can you please confirm if the issue is fixed on your end on FF Nightly 72.0a1? You can download it from here: https://www.mozilla.org/en-US/firefox/channel/desktop/#nightly

Flags: needinfo?(lilydjwg)

Yes, this issue no longer happens to me with recent nightly (currently 2019-10-31).

Flags: needinfo?(lilydjwg)

Thanks for the info, based on comment 27 and 28 I will mark this as verified fixed.

status-firefox72: fixed → verified

Thanks for fixing this. I don't see the problem in Nightly 72.0a1 (2019-11-04) (64-bit)

Comment on attachment 9101444 [details]
Bug 1588386 - Restore toolbars when fullscreen mode exits due to a tab close.

Fix for a 71 blocker with fullscreen video, verified on Nightly, uplift approved for 71 beta 8, thanks.

Attachment #9101444 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos) from comment #25)

Comment on attachment 9101444 [details]
Bug 1588386 - Restore toolbars when fullscreen mode exits due to a tab close.

Beta/Release Uplift Approval Request

  • User impact if declined: Users can get into a state where their browser UI is missing if the tab in which they're watching fullscreen content closes while still in fullscreen.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 4.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a reasonably small, well contained patch that just adds an additional error handling case.
  • String changes made/needed:

Thank you Mike for filling this.

Flags: needinfo?(ablayelyfondou)

I tested this issue on Ubuntu 16.04, Windows 10 and Mac OS X 10.14 with FF Beta 71.0b8 and I can't reproduce the issue.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
status-firefox71: fixed → verified
Flags: qe-verify+

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.

Keywords: rca-needed
Keywords: rca-needed
Whiteboard: rca - Unhandled exceptions
Whiteboard: rca - Unhandled exceptions → [rca - Unhandled exceptions]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: