Closed Bug 1103836 Opened 5 years ago Closed 5 years ago

[Stingray] No 'mozbrowserafterkeydown' event is dispatched after preventing the default actions of 'keydown' event

Categories

(Core :: DOM: Events, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
2.2 S1 (5dec)

People

(Reporter: gyeh, Assigned: gyeh)

References

Details

(Whiteboard: [ft:conndevices])

Attachments

(2 files, 2 obsolete files)

When an iframe is embedded with remote=true, both parent and child processes recieve a keydown event in the following order:

1) parent, 'mozbrowserbeforekeydown'
2) parent, 'keydown'
3) child, 'keydown'
4) parent, 'mozbrowserafterkeydown' with embeddedCancelled = false.



If preventDefault() is invoked at 2), then the expected results are:

1) parent, 'mozbrowserbeforekeydown'
2) parent, 'keydown'
4) parent, 'mozbrowserafterkeydown' with embeddedCancelled = false.




However, the actual results are:

1) parent, 'mozbrowserbeforekeydown'
2) parent, 'keydown'
4) is missing...
Assignee: nobody → gyeh
Depends on: 989198
Whiteboard: [ft:conndevices]
Target Milestone: --- → 2.2 S1 (5dec)
Attachment #8527480 - Flags: review?(bugs)
Attached patch Patch 2(v1): Mochitest. (obsolete) — Splinter Review
Attachment #8527481 - Flags: review?(bugs)
tests are added to verify results.
(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #0) 
> If preventDefault() is invoked at 2), then the expected results are:
> 
> 1) parent, 'mozbrowserbeforekeydown'
> 2) parent, 'keydown'
> 4) parent, 'mozbrowserafterkeydown' with embeddedCancelled = false.
Why embeddedCancelled = false here?
Comment on attachment 8527480 [details] [diff] [review]
Patch 1(v1): dispatch after event.

>   if (defaultPrevented) {
>     *aStatus = nsEventStatus_eConsumeNoDefault;
>-    DispatchAfterKeyboardEventInternal(chain, aEvent,
>-                                       aEvent.mFlags.mDefaultPrevented, chainIndex);
>-
>+    DispatchAfterKeyboardEventInternal(chain, aEvent, false, chainIndex);
Ok, I understand aEvent.mFlags.mDefaultPrevented -> false change


>+  // Actual key event is default-prevented, so dispatch after events.
>+  if (aEvent.mFlags.mDefaultPrevented) {
>+    DispatchAfterKeyboardEventInternal(chain, aEvent, !targetIsIframe, chainIndex);
But I don't understand why !targetIsIframe is passed, and not true.


Please explain.
Attachment #8527481 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #5)
> >+  // Actual key event is default-prevented, so dispatch after events.
> >+  if (aEvent.mFlags.mDefaultPrevented) {
> >+    DispatchAfterKeyboardEventInternal(chain, aEvent, !targetIsIframe, chainIndex);
> But I don't understand why !targetIsIframe is passed, and not true.
> 
> 
> Please explain.

(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #0)
> If preventDefault() is invoked at 2), then the expected results are:
> 
> 1) parent, 'mozbrowserbeforekeydown'
> 2) parent, 'keydown'
  
|targetIsIframe| is true at this time.

Since the default action is prevented by embedder, the value of attribute 'embeddedCancelled' in 'mozbrowserafterkeydown' should be false, which equals to |!targetIsIframe|.

> 4) parent, 'mozbrowserafterkeydown' with embeddedCancelled = false.
> 




If preventDefault() is invoked at 3), then the expected results are:

1) parent, 'mozbrowserbeforekeydown'
2) parent, 'keydown'
3) child, 'keydown'   ->   |targetIsIframe| is false at this time. Since the default action is prevented by embedded iframe, the value of attribute 'embeddedCancelled' should be true, which equals to |!targetIsIframe|.
4) parent, 'mozbrowserafterkeydown' with embeddedCancelled = true.
Comment on attachment 8527480 [details] [diff] [review]
Patch 1(v1): dispatch after event.

I see.

Could you please add a comment to the code why !targetIsIframe is used.
With that, r+
Attachment #8527480 - Flags: review?(bugs) → review+
Comments are added.
Attachment #8527480 - Attachment is obsolete: true
Attachment #8528875 - Flags: review+
Attachment #8527481 - Attachment is obsolete: true
Attachment #8528876 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/745662839b5d
https://hg.mozilla.org/mozilla-central/rev/7dd682deff45
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.