Open Bug 1026865 Opened 5 years ago Updated 3 years ago

Window is occasionally hidden upon exiting a full screen HTML5 video

Categories

(Core :: Widget: Win32, defect)

31 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

()

REOPENED
mozilla36

People

(Reporter: drunkenf00l, Assigned: AtKr2, Mentored, NeedInfo)

References

Details

(Whiteboard: [good first bug][lang=c++])

Attachments

(3 files)

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.
@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
(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)
(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.
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)
(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)
Mentor: jmathies, tabraldes, netzen
Whiteboard: [good first bug][lang=c++]
I'd like to be assigned to this bug.This is my first bug.Can anyone assign me,please ?
There you go - feel free to reach out to the mentors if/when you need help, and good luck!
Assignee: nobody → harshvd95
I disabled flash and tried to open the video.I found nothing unusual.The video worked fine in 720p in my Firefox browser.
I only provided that video link as an example. I've seen the issue on other HTML 5 video even outside of YouTube.
Originally it was(in nsWindow.cpp):  if (mOldSizeMode == nsSizeMode_Normal)
and then it was changed to if (mOldSizeMode == nsSizeMode_Normal || mOldSizeMode == nsSizeMode_Maximized)
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 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+
Thanks for the patch Harsh, and nice work!
https://hg.mozilla.org/integration/mozilla-inbound/rev/53244356f497
Target Milestone: --- → mozilla36
https://hg.mozilla.org/mozilla-central/rev/53244356f497
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Depends on: 1088274
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 → ---
Backout URL coming later today, tree is currently closed.
Happened to me again today for the first time in a while. Firefox 36.0.
Assignee: harshvd95 → nobody
Up for grabs if anyone wants to take a look
Hi Brian, I want to have a look into it. Please assign it to me.
Flags: needinfo?(netzen)
Great, done! :)
Assignee: nobody → atkr.oss
Flags: needinfo?(netzen)
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)
(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)
(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)
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)
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.
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.
(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)
Since this bug hasn't been closed as INCOMPLETE, is it something that still needs to be addressed?
You need to log in before you can comment on or make changes to this bug.