Closed
Bug 969247
Opened 9 years ago
Closed 9 years ago
Get rid of related code of NS_VK_ENTER and nsIDOMKeyEvent::DOM_VK_ENTER
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 1 obsolete file)
3.73 KB,
patch
|
smaug
:
review+
smaug
:
superreview+
|
Details | Diff | Splinter Review |
97.13 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
VK_ENTER is never dispatched by widget. We should remove all handlers of them and test. Note that I don't think that we should remove KeyboardEvent.DOM_VK_ENTER for preventing JS error.
Comment 1•9 years ago
|
||
What kind of JS errors? KeyboardEvent.DOM_VK_ENTER will not throw even if KeyboardEvent.DOM_VK_ENTER is undefined.
Assignee | ||
Comment 2•9 years ago
|
||
Oh, really?
Comment 3•9 years ago
|
||
You can easily test the behavior by typing "KeyboardEvent.WHATEVER_NOT_PRESENT" into Web Console.
Comment 4•9 years ago
|
||
But you'll get undefined if you assign that to some js variable and use that then later, yet expected it to be a number.
Comment 5•9 years ago
|
||
Again, the loose type system will ignore type errors. JavaScript will convert undefined to NaN. Nobody will write a code such as "KeyboardEvent.DOM_VK_ENTER * 3" to calculate the answer to the ultimate question of life, the universe, and everything. The most possible usage would be: switch (event.keyCode) { ... case KeyboardEvent.DOM_VK_ENTER: ... } which is already dead anyway because we have never generated a keyboard event with DOM_VK_ENTER.
Assignee | ||
Comment 6•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=0b7cb1484ccb This patch just removes VK_ENTER handlers/dispatchers or replace them with VK_RETURN. I'm not familiar with marionette part, though...
Attachment #8374552 -
Flags: review?(bugs)
Assignee | ||
Comment 7•9 years ago
|
||
This is removing VK_ENTER both from nsIDOMKeyEvent and KeyEvent.webidl. If we keep not removing from webidl, I'll remove the change of webidl part before landing.
Attachment #8374553 -
Flags: superreview?(bugs)
Attachment #8374553 -
Flags: review?(bugs)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8374552 [details] [diff] [review] part.1 Remove or replace DOM_VK_ENTER and NS_VK_ENTER users Oops, I realized that I missed catching *.jsm.
Attachment #8374552 -
Flags: review?(bugs)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8374552 -
Attachment is obsolete: true
Attachment #8374709 -
Flags: review?(bugs)
Comment 10•9 years ago
|
||
Comment on attachment 8374553 [details] [diff] [review] part.2 Remove DOM_VK_ENTER and NS_VK_ENTER definition Tiny bit scary, but I guess we can try.
Attachment #8374553 -
Flags: superreview?(bugs)
Attachment #8374553 -
Flags: superreview+
Attachment #8374553 -
Flags: review?(bugs)
Attachment #8374553 -
Flags: review+
Comment 11•9 years ago
|
||
Comment on attachment 8374709 [details] [diff] [review] part.1 Remove or replace DOM_VK_ENTER and NS_VK_ENTER users In browser/components/tabview/search.js + if (event.keyCode == event.DOM_VK_RETURN && + (matches.length > 0 || others.length > 0)) { + thsis.hide(event); thsis Are we missing some tests if that passed on try :/
Attachment #8374709 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 12•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #11) > Comment on attachment 8374709 [details] [diff] [review] > part.1 Remove or replace DOM_VK_ENTER and NS_VK_ENTER users > > In browser/components/tabview/search.js > > + if (event.keyCode == event.DOM_VK_RETURN && > + (matches.length > 0 || others.length > 0)) { > + thsis.hide(event); > thsis > > Are we missing some tests if that passed on try :/ Thank you, nice catch! https://hg.mozilla.org/integration/mozilla-inbound/rev/e947cee33a92 https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e9eba7dd1f
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e947cee33a92 https://hg.mozilla.org/mozilla-central/rev/e2e9eba7dd1f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Updated•9 years ago
|
Keywords: dev-doc-needed
Updated•9 years ago
|
Comment 14•9 years ago
|
||
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.keyCode#Constants_for_keyCode_value https://developer.mozilla.org/en-US/Firefox/Releases/30/Site_Compatibility#KeyboardEvent.DOM_VK_ENTER_has_been_removed https://blog.mozilla.org/addons/2014/05/27/compatibility-for-firefox-30/
Keywords: dev-doc-needed → dev-doc-complete
Updated•4 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•