Closed
Bug 630276
Opened 12 years ago
Closed 12 years ago
Neither exiting full-screen video nor closing full-screen window restore menubar/dock
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b12
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: joe, Assigned: mmm)
References
()
Details
(Keywords: regression, Whiteboard: [softblocker])
Attachments
(1 file)
1.50 KB,
patch
|
jaas
:
review+
mstange
:
review+
|
Details | Diff | Splinter Review |
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).
Comment 1•12 years ago
|
||
Does this happen on trunk too?
Reporter | ||
Comment 2•12 years ago
|
||
Sorry I didn't specify - this *is* trunk.
Updated•12 years ago
|
Version: 1.9.2 Branch → Trunk
Updated•12 years ago
|
Keywords: regression,
regressionwindow-wanted
Updated•12 years ago
|
blocking2.0: ? → final+
Whiteboard: [softblocker]
Assignee | ||
Comment 4•12 years ago
|
||
issue also seems to happen if you close a window while in full screen mode.
Assignee | ||
Comment 5•12 years ago
|
||
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
Assignee | ||
Comment 6•12 years ago
|
||
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.
![]() |
||
Updated•12 years ago
|
Keywords: regressionwindow-wanted
![]() |
||
Updated•12 years ago
|
Summary: Exiting full-screen video doesn't restore menubar/dock → Neither exiting full-screen video nor closing full-screen window restore menubar/dock
![]() |
||
Updated•12 years ago
|
Component: Video/Audio Controls → General
Product: Toolkit → Core
QA Contact: video.audio → general
![]() |
||
Updated•12 years ago
|
Component: General → Graphics
QA Contact: general → thebes
Updated•12 years ago
|
blocking2.0: final+ → ---
OS: Mac OS X → Windows 7
![]() |
||
Comment 7•12 years ago
|
||
Gavin, why did this become a non-blocking Windows bug?
Comment 8•12 years ago
|
||
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() ?
Comment 9•12 years ago
|
||
(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.
Comment 10•12 years ago
|
||
Hmm, is it possible that something in fullscreen video UI relies on widget to be deleted almost immediately after hiding the window?
Updated•12 years ago
|
blocking2.0: --- → final+
OS: Windows 7 → Mac OS X
Assignee | ||
Comment 11•12 years ago
|
||
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.
Comment 12•12 years ago
|
||
Moving it to Destroy sounds like a good idea.
Assignee | ||
Comment 13•12 years ago
|
||
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?
Assignee | ||
Updated•12 years ago
|
Attachment #511203 -
Flags: review? → review?(mstange)
Comment 14•12 years ago
|
||
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+
Assignee | ||
Comment 15•12 years ago
|
||
(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?
Comment 16•12 years ago
|
||
Better wait for Josh to give it another rubber stamp; he's the overlord here.
Comment 17•12 years ago
|
||
Comment on attachment 511203 [details] [diff] [review] Patch v1, moves isFullscreen check to Destroy. Thanks!
Attachment #511203 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 18•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/9e27e7d04699
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
![]() |
||
Updated•12 years ago
|
Target Milestone: --- → mozilla2.0b12
You need to log in
before you can comment on or make changes to this bug.
Description
•