Closed
Bug 691583
Opened 13 years ago
Closed 13 years ago
Dispatch event when restricted key input occurs in DOM full-screen mode
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
(Keywords: dev-doc-needed)
Attachments
(2 files, 2 obsolete files)
26.08 KB,
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
16.72 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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 | ||
Updated•13 years ago
|
Assignee | ||
Comment 2•13 years ago
|
||
Patch v1 is based on top of the patch in bug 694204.
Depends on: 694204
Comment 3•13 years ago
|
||
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-
Assignee | ||
Comment 4•13 years ago
|
||
Using nsPLDOMEvent and with chrome mochitest of the "MozShowFullScreenWarning" event.
Attachment #568234 -
Attachment is obsolete: true
Attachment #568925 -
Flags: review?(Olli.Pettay)
Comment 5•13 years ago
|
||
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+
Assignee | ||
Comment 6•13 years ago
|
||
Patch ready for landing.
Attachment #568925 -
Attachment is obsolete: true
Attachment #569271 -
Flags: review+
Assignee | ||
Comment 7•13 years ago
|
||
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.
Assignee | ||
Updated•13 years ago
|
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
Comment 8•13 years ago
|
||
(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.
Assignee | ||
Comment 9•13 years ago
|
||
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.
Assignee | ||
Comment 10•13 years ago
|
||
Patch v3: https://hg.mozilla.org/integration/mozilla-inbound/rev/e269b5ec522a
Standardize on Unix EOL: https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6054dd6e08
Target Milestone: --- → mozilla10
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 13•13 years ago
|
||
In order to write the associated documentation, I have a small question: is this event available to extension authors?
You need to log in
before you can comment on or make changes to this bug.
Description
•