Closed
Bug 708553
Opened 13 years ago
Closed 13 years ago
Hovered element state is not relinquished when entering and exiting fullscreen mode
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: rob, Assigned: cpearce)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
The hover state of the element that is clicked to envoke fullscreen mode (eg. adding a black border around a button) is not relinguished after entering fullscreen even though the mouse may not be over the element (the border remains). This behaviour continues to exist even upon exiting fullscreen mode (the border remains) and can only be resolved by placing the mouse over the original element and removing it again to reset its hover state (no more border).
Demo:
This behaviour happens for me with any code but the easiest example to show is the cross-browser demo: http://html5-demos.appspot.com/static/fullscreen.html
Steps to reproduce:
1. Create an obvious hover state for the button or element used to envoke fullscreen mode, or use the demo link above
2. Hover over said element, notice hover state (black border on the above demo), and click to envoke fullscreen
3. Notice hover state of recently clicked element has not been reset
4. Exit fullscreen mode
5. Notice hover state of recently clicked element has not been reset
6. Place mouse over element and then remove it from the element
7. Notice hover state has been relinquished
Assignee | ||
Comment 1•13 years ago
|
||
Though F11 full-screen mode correctly relinquishes :hover state. Interesting...
Comment 3•13 years ago
|
||
Do we just need to dispatch a synth mouse move so hover gets updated properly?
Comment 4•13 years ago
|
||
cpearce, tn: Do you have some tips for David for fixing this bug?
Assignee: nobody → david.c.seifried
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 5•13 years ago
|
||
Not sure, I haven't looked into this. It would be worth trying Timothy Nickel's suggestion from comment 3 (I guess after we call SetFullScreen [1]). It's a mystery to me why this doesn't occur in F11 full-screen mode though.
Olli Petay or bz may also have some ideas.
[1] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#8483
Comment 6•13 years ago
|
||
:hover/:active etc state handling is bz' and dbaron's territory
Comment 7•13 years ago
|
||
More details for how to do comment 3. To schedule a synth mouse move to be dispatched you want the presshell (nsIPresShell) of the document and then call SynthesizeMouseMove on the presshell. Let me know if you need more info.
Comment 8•13 years ago
|
||
Why doesn't our existing synthetic mouse move code handle this case?
Comment 9•13 years ago
|
||
That's my question too. There should be reflow happening when we enter fullscreen mode, and that shuld trigger posting of synth mouse events. Is that happening?
Assignee | ||
Comment 10•13 years ago
|
||
I suspect I'm hitting this bug, so I tried adding a call to SynthesizeMouseMove on the PresShell after we enter fullscreen and it doesn't fix this bug. By adding logging I can tell there's already a SynthesizeMouseMove happening on the corresponding PresContext anyway.
Comment 11•13 years ago
|
||
Can you remind me what we do with the DOM and styles and such in order to achieve fullscreen mode?
Assignee | ||
Comment 12•13 years ago
|
||
The full-screen element has these styles applied to it:
http://mxr.mozilla.org/mozilla-central/source/layout/style/ua.css#249
And ancestors, including containing iframes and ancestors in parent frames, get these styles applied to them:
http://mxr.mozilla.org/mozilla-central/source/layout/style/full-screen-override.css#39
http://mxr.mozilla.org/mozilla-central/source/layout/style/ua.css#264
We apply the styles before we set the top-level window into fullscreen mode.
Comment 13•13 years ago
|
||
It appears this is a pre-existing problem with the old fullscreen mode (f11) as just that doesn't update hover properly.
Comment 14•13 years ago
|
||
It seems like the second part of the transition, when the tabbar/urlbar is slide off screen, is when the problem happens as hover does seem to be updated during the first part (going fullscreen, getting rid of the titlebar & menubar).
Assignee | ||
Comment 15•13 years ago
|
||
So if I turn on DEBUG_MOUSE_LOCATION in nsPresShell.cpp and observe the calls to PresShell::ProcessSynthMouseMoveEvent() [1], it appears that when we enter fullscreen mPresShell::mMouseLocation is not being updated to reflect the new screen size, i.e. we're constantly calling ProcessSynthMouseMoveEvent, but mMouseLocation is the same before and after the transition to fullscreen mode.
Once we've entered fullscreen and moved the mouse, mMouseLocation gets updated to reflect the correct position, but even then the :hover state isn't updated on the formerly hovered element until you mouseover said element.
[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#5372
Comment 16•13 years ago
|
||
I guess the OS won't send a mouse move if our window moves or changes size? That makes sense. Maybe when we get one of these size/move events in the widget code we should also grab the current mouse position and send a 'fake' mouse event so that we can update the mouse position.
Assignee | ||
Comment 18•13 years ago
|
||
Turns out the problem is that the <browser> element is getting a position:fixed rule applied to it when we enter fullscreen mode, which is causing a reframe, which causes the nsEventStateManager to be recreated, and the new nsEventStateManager doesn't have references to the old one's hover element, so we don't reset the hover state when the mouse moves thanks to a synthetic mouse event.
If we just don't apply position:fixed to the xul elements, we avoid the reframe, and everything works as expected.
Attachment #599040 -
Flags: review?(roc)
Comment 19•13 years ago
|
||
Would you still run into issues if you trigger fullscreen on something in a subframe? I don't see why XUL is special here...
Assignee | ||
Comment 20•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #19)
> Would you still run into issues if you trigger fullscreen on something in a
> subframe? I don't see why XUL is special here...
Hmmm, yes, it appears so... What should the rule be?
Comment 21•13 years ago
|
||
The right fix would seem to be to deal with preserving whatever state we need across reframes...
Assignee | ||
Updated•13 years ago
|
Attachment #599040 -
Flags: review?(roc) → review-
I guess the patch is a bit of a workaround. It works because the <browser> element which goes fullscreen in the chrome document doesn't need any styling, since the chrome document responds to the fullscreen event by changing its layout to make the <browser> fullscreen anyway.
The underlying bug we're working around is that changing an iframe to position:fixed reframes the subdocument, which destroys and recreates the subdocument's presentation, which loses information about hover state. Maybe we should fix the underlying bug so that frame reconstruction doesn't destroy/recreate the subdocument presentation? That's possible I guess, but tricky. At least it would require view reparenting.
Comment 23•13 years ago
|
||
Keeping the old presentation around doesn't seem too bad, but if we are reframing because the iframe was made display: none how exactly do we determine that we should throw away the presentation?
(In reply to Timothy Nikkel (:tn) from comment #23)
> Keeping the old presentation around doesn't seem too bad, but if we are
> reframing because the iframe was made display: none how exactly do we
> determine that we should throw away the presentation?
How about making the call to HideViewer from nsSubDocumentFrame::DestroyFrom happen off nsContentUtils::AddScriptRunner, and in that scriptrunner check to see if the <iframe> element has no frame before calling HideViewer?
Comment 25•13 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (away February 21 to March 17) from comment #24)
> How about making the call to HideViewer from nsSubDocumentFrame::DestroyFrom
> happen off nsContentUtils::AddScriptRunner, and in that scriptrunner check
> to see if the <iframe> element has no frame before calling HideViewer?
Wouldn't we need to do it at the next refresh driver tick because that is when we would actually do the async work to recreate the frame? That would make the bookkeeping to keep track of the work that needs to be done a little more involved.
Just have the scriptrunner do a FlushPendingNotifications(Flush_Frames) before checking whether a frame exists. Sure, that disables lazy frame construction when a subdocument frame is destroyed, but that probably doesn't matter.
Comment 27•13 years ago
|
||
Or we could have add a refresh observer when a subdocument frame gets destroyed to do that work.
Leaving the presentation unhidden for that long might be risky.
Comment 29•13 years ago
|
||
Hadn't thought of it that way. What kind of risks are you thinking of?
Assignee | ||
Updated•13 years ago
|
Blocks: CVE-2012-3988
Assignee | ||
Comment 30•13 years ago
|
||
So... What's the best way to accomplish preserving the presentation across reframes? We've talked about only calling HideViewer() in a script runner if the subdocframe is display:none, but what should we do in the non-display:none case? What state do we need to stash, where and when should we stash it, and where should we restore it?
Comment 31•13 years ago
|
||
Just off the top of my head we'd take the root view of the subdocument and stash it on the content node for the subdocument frame. We'd check before we called ShowViewer after reconstructing the subdocument frame if the content node has a stashed root view for a subdocument. We'd do like comment 26 and kick off a scriptrunner when we do the stashing, after the flush the scriptrunner would check if the content node for the subdocument frame had a frame, if it didn't it would unstash and call HideViewer.
Comment 32•13 years ago
|
||
Possibly related: Youtube's WebM player has a full screen button with a title attribute. Clicking the full screen button triggers the tooltip, which only disappears upon input. So it sits there interrupting the video until you do something.
Not sure if that is in the scope of this bug or whether tooltips are too far unrelated to hover states.
Assignee | ||
Comment 33•13 years ago
|
||
Lozzy: I imagine tooltips are implemented using the hover/mouseover status, so I'd guess they're related. Thanks for commenting.
Assignee | ||
Comment 34•13 years ago
|
||
Since I'm reliably informed that I won't get a chance to work on anything except OMG MOBILE VIDEO for a while, so I think we should take this simple fix rather than waiting for me to free up enough to cycles to fix the reframing code.
This patch just resets the hover state when the prescontext is destroyed. The synthetic mouse move after reframing ensures hover state is reapplied correctly after the reframe in the new presentation.
Attachment #599040 -
Attachment is obsolete: true
Attachment #624637 -
Flags: review?(bzbarsky)
Comment 35•13 years ago
|
||
Comment on attachment 624637 [details] [diff] [review]
Patch: reset hover content when prescontext destroyed
r=me.
Attachment #624637 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 36•13 years ago
|
||
Target Milestone: --- → mozilla15
Comment 37•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•13 years ago
|
No longer blocks: CVE-2012-3988
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•