Last Comment Bug 706599 - Handle no default in gonk key dispatching
: Handle no default in gonk key dispatching
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: unspecified
: All Gonk (Firefox OS)
: -- normal (vote)
: mozilla11
Assigned To: Michael Wu [:mwu]
:
Mentors:
Depends on:
Blocks: 709468
  Show dependency treegraph
 
Reported: 2011-11-30 11:57 PST by Michael Wu [:mwu]
Modified: 2011-12-13 10:56 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Handle no default in gonk key dispatching (1.68 KB, patch)
2011-11-30 11:57 PST, Michael Wu [:mwu]
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Michael Wu [:mwu] 2011-11-30 11:57:14 PST
Created attachment 578035 [details] [diff] [review]
Handle no default in gonk key dispatching

This makes nsEventStatus_eConsumeNoDefault get handled correctly.
Comment 1 Justin Lebar (not reading bugmail) 2011-11-30 12:13:22 PST
Comment on attachment 578035 [details] [diff] [review]
Handle no default in gonk key dispatching

> +    event.flags |= flags;

It would be clearer to do |event.flags = flags|.  Unless event.flags may be non-zero at this point?

For my edification, exactly what problem does this fix?  Is it that if you cancel the keydown, we shouldn't send the keypress?

btw, can you please set git to use more context for diffs?  (Bonus points for using --patience.)
Comment 2 Michael Wu [:mwu] 2011-12-02 10:02:06 PST
(In reply to Justin Lebar [:jlebar] from comment #1)
> Comment on attachment 578035 [details] [diff] [review] [diff] [details] [review]
> Handle no default in gonk key dispatching
> 
> > +    event.flags |= flags;
> 
> It would be clearer to do |event.flags = flags|.  Unless event.flags may be
> non-zero at this point?
> 

It is set to NS_EVENT_FLAG_TRUSTED.

> For my edification, exactly what problem does this fix?  Is it that if you
> cancel the keydown, we shouldn't send the keypress?
> 

Something like that. I don't remember the details, but it's the right thing to do and all the widget backends should be doing it.
Comment 3 Philipp von Weitershausen [:philikon] 2011-12-12 14:46:13 PST
This patch has review and still applies to m-c. Unless there are any objections, I will land this.
Comment 4 Michael Wu [:mwu] 2011-12-13 10:56:23 PST
https://hg.mozilla.org/mozilla-central/rev/16d41bf6df46

Note You need to log in before you can comment on or make changes to this bug.