Closed Bug 729872 Opened 8 years ago Closed 8 years ago

Window deactivate while in HTML5 fullscreen causes text fields to not show cursor or selection

Categories

(Core :: General, defect)

11 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla13
Tracking Status
firefox10 --- wontfix
firefox11 --- wontfix
firefox12 --- verified
firefox13 --- verified
firefox-esr10 - wontfix

People

(Reporter: pauly, Assigned: cpearce)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Steps to reproduce:
1. Enable html5 http://www.youtube.com/html5
2. Open http://www.youtube.com/watch?v=R8Or0qxb0-k
3. Enter youtube fullscreen
4. Press ALT+TAB

Actual results:
Youtube leaves fullscreen mode. Location bar, search bar become unresponsive

Expected results:
Location bar, search bar should work properly
Bisect with  full-screen-api.enabled=true;

Regression window(m-c),
Cannot reproduce:
http://hg.mozilla.org/mozilla-central/rev/e6893e6c883f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111104 Firefox/10.0a1 ID:20111104020439
Can reproduce:
http://hg.mozilla.org/mozilla-central/rev/5ebd59b5a94a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111104 Firefox/10.0a1 ID:20111104112939
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e6893e6c883f&tochange=5ebd59b5a94a


Regression window(m-i),
Cannot reproduce:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4f57f89653df
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111103 Firefox/10.0a1 ID:20111103223340
Can reproduce:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e7e0950aec83
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111103 Firefox/10.0a1 ID:20111104003338
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4f57f89653df&tochange=e7e0950aec83
Suspect: Bug 685402
Blocks: 685402
Product: Firefox → Core
QA Contact: general → general
Blocks: 545812
Assignee: nobody → cpearce
Text fields still work, we just don't show the caret or highlight the text selection. Toggling F11 fullscreen mode makes text fields behave as expected again.
Summary: HTML5 fullscreen loosing focus causes text entry fields to become unresponsive → Window deactivate while in HTML5 fullscreen causes text fields to not show cursor & selection
Summary: Window deactivate while in HTML5 fullscreen causes text fields to not show cursor & selection → Window deactivate while in HTML5 fullscreen causes text fields to not show cursor or selection
So there's two problems here:

1. nsFocusManager dispatches "deactivate" synchronously, and then does some more stuff related to blurring the focused window. The "deactivate" listener in browser.js exits fullscreen, which lowers and raises the focused window, which messes up the blur logic, and is what's causing the text widgets to not work upon focus after deactivate. Making the "deactivate" listener exit fullscreen in a timeout works around this issue, and text inputs work after deactivate in fullscreen. This also fixes bug 726474.

2. nsGlobalWindow::SetFullScreen() (on Windows at least) causes the top level browser window to be lowered and then raised. So after the top-level window has been blurred from alt+tab, we'll call SetFullScreen(false) to exit fullscreen mode, which will lower and then raise the window, causing the window to be drawn as if it's been raised (focused, and the window manager's active window), even though it's not the window manager's active window.

I'll spin off problem 2 into another bug.
Blocks: 726474
Attached patch Patch (obsolete) — Splinter Review
Fix as I describe in comment 1, exit fullscreen on deactivate asynchronously, so as to not confuse the focus manager's blur code.
Attachment #601084 - Flags: review?(dao)
Depends on: 731029
Comment on attachment 601084 [details] [diff] [review]
Patch

Please implement FullScreen.handleEvent and call window.addEventListener("deactivate", this)
Attachment #601084 - Flags: review?(dao) → review-
Attached patch Patch v2Splinter Review
Attachment #601084 - Attachment is obsolete: true
Attachment #601740 - Flags: review?(dao)
Comment on attachment 601740 [details] [diff] [review]
Patch v2

Much nicer, thanks.

>   exitDomFullScreen : function(e) {
>     document.mozCancelFullScreen();
>   },

The 'e' argument is obsolete now.

>+  handleEvent: function (event) {
>+    switch (event.type) {
>+      case "deactivate":
>+        // We must call exitDomFullScreen asynchronously, since "deactivate" is
>+        // dispatched in the middle of the focus manager's window lowering code,
>+        // and the focus manager gets confused if we exit fullscreen mode in the
>+        // middle of window lowering. See bug 729872.
>+        setTimeout(this.exitDomFullScreen, 0);

Please make this this.exitDomFullScreen.bind(this) even though it doesn't currently matter given how exitDomFullScreen is implemented.

>+      window.addEventListener("deactivate", this, true);

Any reason why this isn't just window.addEventListener("deactivate", this)? I.e. why is this a capturing listener?
Attachment #601740 - Flags: review?(dao) → review+
(In reply to Dão Gottwald [:dao] from comment #7)
> Comment on attachment 601740 [details] [diff] [review]
> >+      window.addEventListener("deactivate", this, true);
> 
> Any reason why this isn't just window.addEventListener("deactivate", this)?
> I.e. why is this a capturing listener?

Hmmm, I don't think it needs to be, I'll make it non-capturing.
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/9b65b4ac15cd
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Verified fixed on FF13:
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120312 Firefox/13.0a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120304 Firefox/13.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20120311 Firefox/13.0a1
Status: RESOLVED → VERIFIED
Comment on attachment 601740 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: Exiting full-screen by ALT+TABing to another window will cause text fields in Firefox UI to appear to be unfocused, even though they are; so the user may not realise they can type in the Awesome/search bars.
Testing completed (on m-c, etc.): Been on m-c for a while.
Risk to taking this patch (and alternatives if risky): Pretty low risk, just makes an event listener async.
String changes made by this patch: None.

Once the Mar 13 merge is complete, this fix will already be on Aurora, so I'm only requesting approval for beta.
Attachment #601740 - Flags: approval-mozilla-beta?
Comment on attachment 601740 [details] [diff] [review]
Patch v2

[Triage Comment]
Issue with a recent new feature and early enough in Beta 12 to take a fix like this.
Attachment #601740 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
It would be good to fix this bug on ESR10 as well.
We don't expect this to be a major pain point for ESR users, and it's not a critical security/stability fix - let's let this ride the train and get fixed in ESR17.
Still an issue on FF 12b2: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0
Verified fixed on FF 12b3 on Win 7/64, Ubuntu 11.10 and Mac OS X 10.6
You need to log in before you can comment on or make changes to this bug.