Neither exiting full-screen video nor closing full-screen window restore menubar/dock

RESOLVED FIXED in mozilla2.0b12

Status

()

defect
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: joe, Assigned: mmm)

Tracking

({regression})

Trunk
mozilla2.0b12
x86
macOS
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker], )

Attachments

(1 attachment)

If you full-screen a video, then exit full-screen by pushing escape, the OS X menubar and dock remain hidden. Switching windows from Firefox to another application will restore them; getting them back permanently involves switching windows "several times" (haven't quite figured that part out yet, though the dock and menubar do eventually come back).
Does this happen on trunk too?
Sorry I didn't specify - this *is* trunk.
Version: 1.9.2 Branch → Trunk
This is probably a softblocker.
blocking2.0: --- → ?
blocking2.0: ? → final+
Whiteboard: [softblocker]
issue also seems to happen if you close a window while in full screen mode.
taking it over for now.

tested it on nightlies, looks like it breaks in 2011-01-19 but is good in 2011-01-18, this is the corresponding window:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b521854c006d&tochange=809aded51aad
Assignee: nobody → mars.martian+bugmail
It looks like http://hg.mozilla.org/mozilla-central/rev/acb48b922e91 causes the problem, not sure why though. CC'ing Olli to see if he can shed some light on the problem.
Summary: Exiting full-screen video doesn't restore menubar/dock → Neither exiting full-screen video nor closing full-screen window restore menubar/dock
Component: Video/Audio Controls → General
Product: Toolkit → Core
QA Contact: video.audio → general
Component: General → Graphics
QA Contact: general → thebes
blocking2.0: final+ → ---
OS: Mac OS X → Windows 7
Gavin, why did this become a non-blocking Windows bug?
huh, I have no idea how Bug 625311 could have caused that problem.
We really can't back that out.

How does fullscreen video work? Do we create a new widget for it?
What keeps that widget alive? Are we not calling nsBaseWidget::OnDestroy() ?
(In reply to comment #8)
> huh, I have no idea how Bug 625311 could have caused that problem.
> We really can't back that out.
...or we'd need to fix the issue in some other way.
Hmm, is it possible that something in fullscreen video UI relies on widget to be
deleted almost immediately after hiding the window?
blocking2.0: --- → final+
OS: Windows 7 → Mac OS X
From debugging this morning, it seems like something holds a reference to the
nsThebesDeviceContext which isn't released normally but is later GC'ed and
destroyed. After this, the widget is finally destroyed and consequently the
menubar and dock are restored.

Instead of solving that pointer problem, we could move the |if (mFullscreen)|
block from ~nsCocoaWindow to nsCocoaWindow::Destroy. Not sure how risky that
would be though.

CC'ing Josh because it seems like he works on that section.
Moving it to Destroy sounds like a good idea.
Patch works in both cases, when closing a full screen window and when closing a full screen video. (In the second case, the video and tab are still around)

Running this through on try to make sure it doesn't break anything.
Attachment #511203 - Flags: review?
Attachment #511203 - Flags: review? → review?(mstange)
Comment on attachment 511203 [details] [diff] [review]
Patch v1, moves isFullscreen check to Destroy.

Looks good, thanks!

At some point we should make sure that we also handle the window being hidden and re-shown during its lifetime, and only hide the OS chrome while the fullscreen window is visible, but that can wait. Fullscreenable windows are currently guaranteed to be destroyed right after being hidden, so this will work for now.
Attachment #511203 - Flags: review?(mstange)
Attachment #511203 - Flags: review?(joshmoz)
Attachment #511203 - Flags: review+
(In reply to comment #14)
> Comment on attachment 511203 [details] [diff] [review]
> Patch v1, moves isFullscreen check to Destroy.
> 
> Looks good, thanks!
> 
> At some point we should make sure that we also handle the window being hidden
> and re-shown during its lifetime, and only hide the OS chrome while the
> fullscreen window is visible, but that can wait. Fullscreenable windows are
> currently guaranteed to be destroyed right after being hidden, so this will
> work for now.

Sweet!

Is your review enough to check this in, or should I wait for Josh to give it a look?
Better wait for Josh to give it another rubber stamp; he's the overlord here.
Comment on attachment 511203 [details] [diff] [review]
Patch v1, moves isFullscreen check to Destroy.

Thanks!
Attachment #511203 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/9e27e7d04699
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
You need to log in before you can comment on or make changes to this bug.