691583 - Dispatch event when restricted key input occurs in DOM full-screen mode

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

Change DOM full-screen mode to instead show the "Press ESC to leave full-screen" warning message when restricted key input occurs, rather than exiting full-screen.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: dispatch showfullscreenwarning chrome event when restricted key pressed in full-screen mode

The secteam decided it was sufficient to just display a warning saying how to exit full-screen when a restricted key was pressed, rather than exit full-screen mode on restricted key press. So this patch changes behaviour to dispatch "showfullscreenwarning" to chrome when a restricted key is pressed. My patch in bug 684625 will listen for this event and display the warning.

This patch also alters the test to reflect that most keys don't cause full-screen exit any more.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Patch v1 is based on top of the patch in bug 694204.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 568234 [details] [diff] [review]
Patch: dispatch showfullscreenwarning chrome event when restricted key pressed in full-screen mode


>+        } else if (IsFullScreenAndRestrictedKeyEvent(mCurrentEventContent, aEvent)) {
>+          // Restricted key press while in DOM full-screen mode. Dispatch
>+          // an event to chrome so it knows to show a warning message
>+          // informing the user how to exit full-screen.
>+          bool defaultActionEnabled = true;
>+          nsPIDOMWindow* outer = doc->GetWindow()->GetOuterWindow();
doc->GetWindow() is the outer window.
But actually, why is window the target of the event and not document.


>+          nsContentUtils::DispatchChromeEvent(doc,
>+                                              outer,
>+                                              NS_ConvertASCIItoUTF16("showfullscreenwarning"),
NS_LITERAL_STRING

>+                                              PR_TRUE, PR_TRUE, &defaultActionEnabled);
The last parameter is optional.


Could you call the event something like MozShowFullScreenWarning or something.
It is after all very mozilla specific.

Also, do we want to dispatch that even synchronously or would async work.
For async
nsRefPtr<nsPLDOMEvent> e = new nsPLDOMEvent(doc, NS_LITERAL_STRING("MozShowFullScreenWarning"), true, true);
e->PostDOMEvent();


I think I'd prefer async event if there isn't need for sync.


I assume there will be tests for the new event in some other bug?

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v2

Using nsPLDOMEvent and with chrome mochitest of the "MozShowFullScreenWarning" event.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 568925 [details] [diff] [review]
Patch v2

>+         nsIDocument *doc = mCurrentEventContent ?
>+                            mCurrentEventContent->OwnerDoc() : nsnull;
>+         if (doc &&
>+            doc->IsFullScreenDoc() &&
Missing space before doc->

Please make sure there are no windows line endings in the patch.
(Some of the fullscreen patches did have)

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v3

Patch ready for landing.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: change EOL to unix

A few of the new test files I created had windows line endings, or even a mixture. This patch normalizes all the tests to Unix line endings.

Comment 8

[Dão Gottwald [:dao]]

(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #0)
> Change DOM full-screen mode to instead show the "Press ESC to leave
> full-screen" warning message when restricted key input occurs, rather than
> exiting full-screen.

Would we, without this patch, exit FS mode when hitting Ctrl? If so, does the patch enable shortcuts such as Ctrl+Tab? This would seem problematic to me.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Yes, this patch will enable ctrl+tab. I am planning to exit full-screen on change-tab (and alt+tab change-app) in bug 685402 to handle this. I haven't figured out the best way to detect change-tab and change-app yet, any ideas you have on how to implement this would be most welcome in bug 685402.

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Patch v3: https://hg.mozilla.org/integration/mozilla-inbound/rev/e269b5ec522a
Standardize on Unix EOL: https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6054dd6e08

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/e269b5ec522a

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Also https://hg.mozilla.org/mozilla-central/rev/ad6054dd6e08

Comment 13

[Jean-Yves Perrier [:teoli]]

In order to write the associated documentation, I have a small question: is this event available to extension authors?