Closed Bug 691583 Opened 8 years ago Closed 8 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: cpearce, Assigned: cpearce)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files, 2 obsolete files)

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.
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.
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #568234 - Flags: review?(Olli.Pettay)
Blocks: 684625
No longer depends on: 684625
Patch v1 is based on top of the patch in bug 694204.
Depends on: 694204
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?
Attachment #568234 - Flags: review?(Olli.Pettay) → review-
Attached patch Patch v2 (obsolete) — Splinter Review
Using nsPLDOMEvent and with chrome mochitest of the "MozShowFullScreenWarning" event.
Attachment #568234 - Attachment is obsolete: true
Attachment #568925 - Flags: review?(Olli.Pettay)
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)
Attachment #568925 - Flags: review?(Olli.Pettay) → review+
Attached patch Patch v3Splinter Review
Patch ready for landing.
Attachment #568925 - Attachment is obsolete: true
Attachment #569271 - Flags: review+
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.
Summary: Show warning message when restricted key input occurs in DOM full-screen mode → Dispatch event when restricted key input occurs in DOM full-screen mode
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/e269b5ec522a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
In order to write the associated documentation, I have a small question: is this event available to extension authors?
Depends on: 729830
You need to log in before you can comment on or make changes to this bug.