Closed
Bug 1026865
Opened 11 years ago
Closed 3 years ago
Window is occasionally hidden upon exiting a full screen HTML5 video
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
INCOMPLETE
mozilla36
People
(Reporter: drunkenf00l, Unassigned, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=c++])
Attachments
(3 files)
1.04 MB,
image/png
|
Details | |
930 bytes,
patch
|
Details | Diff | Splinter Review | |
891 bytes,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
I was viewing an HTML5 video on youtube.com (https://www.youtube.com/watch?v=GLlLQ3LmZWU). Resolution was set to 720p. I entered full screen for a few minutes by pressing the control in the bottom right. I used the control in the bottom right to exit full screen.
Firefox exited full screen mode, but the video continued to play and the window was completely hidden. I managed to use Spy++ and the ShowWindow win32 api to get the window to show again. The window was maximized. The window wasn't maximized when I entered full screen mode.
I attached a screenshot of the window upon unhiding the window with the ShowWindow API.
The window was completely functional after unhiding it.
The window should not have been hidden and should have returned to the state it was in prior to entering full screen.
This has happened to me several times. It does not happen every time. I believe I've hit the same bug in version 30 and perhaps even earlier.
Comment 1•11 years ago
|
||
@jimm, I spoke with the reporter as this was happening, and it appears we're legitimately hiding the window upon exiting HTML5 full screen and failing to re-show it. The only spot I can find where we intentionally hide the window on these code paths is at:
http://dxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#2863
Do you see anything immediately wrong with that code or know what some good debugging steps for this might be? The symptoms seem pretty bad, requiring a task-manager kill to bring firefox back.
Status: UNCONFIRMED → NEW
Component: General → Widget: Win32
Ever confirmed: true
Flags: needinfo?(jmathies)
Product: Firefox → Core
![]() |
||
Comment 2•11 years ago
|
||
(In reply to drunkenf00l from comment #0)
> Created attachment 8441832 [details]
> The window upon being forcefully unhidden
>
> I was viewing an HTML5 video on youtube.com
> (https://www.youtube.com/watch?v=GLlLQ3LmZWU). Resolution was set to 720p. I
> entered full screen for a few minutes by pressing the control in the bottom
> right. I used the control in the bottom right to exit full screen.
(In reply to John Schoenick [:johns] from comment #1)
> @jimm, I spoke with the reporter as this was happening, and it appears we're
> legitimately hiding the window upon exiting HTML5 full screen and failing to
> re-show it. The only spot I can find where we intentionally hide the window
> on these code paths is at:
> http://dxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.
> cpp#2863
>
> Do you see anything immediately wrong with that code or know what some good
> debugging steps for this might be? The symptoms seem pretty bad, requiring a
> task-manager kill to bring firefox back.
Was the reporter talking about the full screen button in the youtube player or fullscreen (F11) for the browser?
Flags: needinfo?(jmathies)
![]() |
||
Comment 3•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #2)
> Was the reporter talking about the full screen button in the youtube player
> or fullscreen (F11) for the browser?
Ok, I'm seeing this now with flash completely disabled. We appear to reuse the base browser window to display the video.
(In reply to John Schoenick [:johns] from comment #1)
> @jimm, I spoke with the reporter as this was happening, and it appears we're
> legitimately hiding the window upon exiting HTML5 full screen and failing to
> re-show it. The only spot I can find where we intentionally hide the window
> on these code paths is at:
> http://dxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.
> cpp#2863
>
> Do you see anything immediately wrong with that code or know what some good
> debugging steps for this might be? The symptoms seem pretty bad, requiring a
> task-manager kill to bring firefox back.
Nothing obvious stands out. The failure would have to come on the follow up MakeFullscreen(false) call, maybe mIsVisible is unexpectedly false there.
Simple dump statements watching state in that method if we can get str to reproduce would be a good starting point.
Comment 4•11 years ago
|
||
Bug 1019379 is different, but has a similar symptom - video playing in a hidden window. needinfo myself to see if adding printf's to that function shows anything useful using the STR in that other bug.
Flags: needinfo?(mhammond)
Comment 5•11 years ago
|
||
(In reply to Mark Hammond [:markh] from comment #4)
> Bug 1019379 is different, but has a similar symptom - video playing in a
> hidden window. needinfo myself to see if adding printf's to that function
> shows anything useful using the STR in that other bug.
In that bug, nsWindow::MakeFullScreen isn't called - presumably as Flash is managing full-screen.
Flags: needinfo?(mhammond)
![]() |
||
Updated•11 years ago
|
Mentor: jmathies, tabraldes, netzen
Whiteboard: [good first bug][lang=c++]
Comment 6•10 years ago
|
||
I'd like to be assigned to this bug.This is my first bug.Can anyone assign me,please ?
Comment 7•10 years ago
|
||
There you go - feel free to reach out to the mentors if/when you need help, and good luck!
Assignee: nobody → harshvd95
Comment 8•10 years ago
|
||
I disabled flash and tried to open the video.I found nothing unusual.The video worked fine in 720p in my Firefox browser.
Reporter | ||
Comment 9•10 years ago
|
||
I only provided that video link as an example. I've seen the issue on other HTML 5 video even outside of YouTube.
Comment 10•10 years ago
|
||
Attachment #8506212 -
Flags: review?(netzen)
Comment 11•10 years ago
|
||
Originally it was(in nsWindow.cpp): if (mOldSizeMode == nsSizeMode_Normal)
and then it was changed to if (mOldSizeMode == nsSizeMode_Normal || mOldSizeMode == nsSizeMode_Maximized)
Comment 12•10 years ago
|
||
Comment on attachment 8506212 [details] [diff] [review]
OR statement added mOldSizeMode == nsSizeMode_Maximized
Review of attachment 8506212 [details] [diff] [review]:
-----------------------------------------------------------------
I think maybe you have 2 patches in your patch queue and you attached only the 2nd.
This patch just adds a space to a line of code.
Check which patches you have applied with hg qapplied.
Then you can hg qpop to pop off the last patch.
Then you can hg qfold patch-name-you-juust-popped.diff to merge it down into the first patch.
Attachment #8506212 -
Flags: review?(netzen)
Comment 13•10 years ago
|
||
Attachment #8506369 -
Flags: review?(netzen)
Comment 14•10 years ago
|
||
Comment on attachment 8506369 [details] [diff] [review]
bug1026865_nsSizeMode.diff
Review of attachment 8506369 [details] [diff] [review]:
-----------------------------------------------------------------
Great, thanks for the patch! I'll push this to try and test it out locally and if it's all good I'll push it to mozilla-inbound.
From there it'll go to mozilla-central within a day and be in Nightly builds.
Attachment #8506369 -
Flags: review?(netzen) → review+
Comment 15•10 years ago
|
||
Comment 16•10 years ago
|
||
Thanks for the patch Harsh, and nice work!
https://hg.mozilla.org/integration/mozilla-inbound/rev/53244356f497
Target Milestone: --- → mozilla36
Comment 17•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 18•10 years ago
|
||
I'm going to re-open this and back it out because of bug 1088274. We'll re-land this once that one is fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•10 years ago
|
||
Backout URL coming later today, tree is currently closed.
Comment 20•10 years ago
|
||
Backed out here:
https://hg.mozilla.org/integration/mozilla-inbound/rev/844ad6f2b2ae
Reporter | ||
Comment 21•10 years ago
|
||
Happened to me again today for the first time in a while. Firefox 36.0.
Updated•10 years ago
|
Assignee: harshvd95 → nobody
Comment 22•10 years ago
|
||
Up for grabs if anyone wants to take a look
Comment 23•10 years ago
|
||
Hi Brian, I want to have a look into it. Please assign it to me.
Flags: needinfo?(netzen)
Comment 25•10 years ago
|
||
Tony,
Earlier fix created a regression and thus was backed out. I would like to know if the issue is still happening. I tried the link in repro steps as well as two more random videos on youtube and could not repro. I could not repro when when starting firefox in safe mode, no add on, while having flash and silverlight disabled.
I see one problem related to fullscreen at https://support.mozilla.org/en-US/questions/1062134 and tracked by https://bugzilla.mozilla.org/show_bug.cgi?id=947854. There is one old problem at https://support.mozilla.org/en-US/questions/979557 also.
It may be naive, but did you check with firefox in safe mode?
When you see this issue, is firefox in full screen mode or normal mode?
Is it happening everywhere, desktop and laptop, or specific to some system?
I am just looking to repro and thus these questions. If you have already discussed it in detail with John Schoenick [:johns] offline, can you forward me that conversation or attach to the bug?
Flags: needinfo?(drunkenf00l)
Comment 26•10 years ago
|
||
(In reply to John Schoenick [:johns] from comment #1)
> @jimm, I spoke with the reporter as this was happening, and it appears we're
> legitimately hiding the window upon exiting HTML5 full screen and failing to
> re-show it. The only spot I can find where we intentionally hide the window
> on these code paths is at:
> http://dxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.
> cpp#2863
>
> Do you see anything immediately wrong with that code or know what some good
> debugging steps for this might be? The symptoms seem pretty bad, requiring a
> task-manager kill to bring firefox back.
I am not able to repro this issue and not getting response from reporter. Would you be able to help me with repro? Trying to fix without confirmed repro would be futile I think.
Flags: needinfo?(john)
Comment 27•10 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #24)
> Great, done! :)
I am not getting repro at my end and not getting response from original poster or people who followed up this issue. What would you suggest as next course of action?
Flags: needinfo?(netzen)
Comment 28•10 years ago
|
||
Possibly playing with code around widget/windows/nsWindow.cpp mIsFullScreen to see if you can find a way to reproduce it more often by changing code. I can't reproduce either out of the box, sorry.
Mentor: netzen
Flags: needinfo?(netzen)
Comment 29•10 years ago
|
||
As per private email, let's leave the request open for more info and please feel free to grab another bug that allows you to reproduce. If we don't get any more reports or info we can resolve as incomplete.
Comment 30•10 years ago
|
||
We will leave this open for next 3 months waiting for more reports of repro and any additional information towards that. Please alert me through NI or other means any time a possible repro is available. On the end of 3 months waiting, this will be closed as INCOMPLETE.
Comment 31•10 years ago
|
||
(In reply to Atul Kumar [:AtKr2] from comment #30)
> We will leave this open for next 3 months waiting for more reports of repro
> and any additional information towards that. Please alert me through NI or
> other means any time a possible repro is available. On the end of 3 months
> waiting, this will be closed as INCOMPLETE.
This sounds fine to me
Flags: needinfo?(john)
Comment 32•8 years ago
|
||
Since this bug hasn't been closed as INCOMPLETE, is it something that still needs to be addressed?
Keywords: good-first-bug
Whiteboard: [good first bug][lang=c++] → [lang=c++]
Comment 33•4 years ago
|
||
This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.
Assignee: atkr.oss → nobody
Status: REOPENED → NEW
Updated•3 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago → 3 years ago
Flags: needinfo?(drunkenf00l)
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•