<input> has focus/blur issues when it has focus and its type changes to/from type=number
Categories
(Core :: Layout: Form Controls, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox27 | --- | unaffected |
firefox28 | --- | wontfix |
firefox29 | --- | wontfix |
firefox30 | --- | wontfix |
firefox74 | --- | fixed |
People
(Reporter: jchen, Assigned: emilio)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [webcompat:p1], [wptsync upstream])
Attachments
(4 files, 2 obsolete files)
The first input in the test case switches its type from 'text' to 'number' in its onfocus handler, and switches back in onblur. However, after it switches to number, it no longer accepts any input. This happens because the focus is still on the parent element rather than on the (newly created) anonymous child element. I think the block at [1] is supposed to fix this problem, but it doesn't actually work because the anonymous child is not bound to the document at that point, so nsFocusManager::SetFocus() fails. SetFocus() wouldn't work in any case, because assuming the anonymous child is focused, the parent input element will get a blur event; during the blur handler, the input type is switched back to text and everything is destroyed; we still don't get a focused number input. I think we have to make some changes to nsFocusManager to fix this. [1] http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsNumberControlFrame.cpp?rev=63a4ad62401a#359
Reporter | ||
Updated•10 years ago
|
Updated•10 years ago
|
Comment 1•9 years ago
|
||
There are other related issues when changing the type. For example, bug 1057858 comment 3. https://jsfiddle.net/5gc7upLj/
Comment 2•9 years ago
|
||
Here is a version with all the current input types: http://jsfiddle.net/5gc7upLj/2/ This confirms that it only occurs with the number type.
Updated•8 years ago
|
Comment 3•8 years ago
|
||
This breaks the "Search by flight number" status functionality on westjet.com's mobile site. (If I had to guess, sites do this because they want the "number" keypad (on mobile), but not the ugly spinners that they can't turn off.)
Comment 4•8 years ago
|
||
Bumping up to P3 because this breaks real sites (not sure anyone uses priority values, but all the same...)
Wasted hours tracking this down. Rather subtle. Reported at https://stackoverflow.com/questions/38770152/firefox-blurs-when-changing-the-input-type where it was blamed on AngularJS. However here's a demo of it happening with jQuery. The field can only be edited once in Firefox. https://jsfiddle.net/BobStein/4mL902vo/7/ Changing an input field from text to number causes the field to spontaneously blur.
Updated•6 years ago
|
Comment 6•6 years ago
|
||
This is quite an annoying issue, as DOM insertion includes changing the type. Our use case is for an INTL currency input: - By default be input type="text" for INTL currency formatting, - On focus/click change to input type="number" to benefit from browser number functionality. - On blur change to input type="text" for INTL currency formatting again. In Reactive UI programming this is much hit upon problem: - https://github.com/facebook/react/issues/11062 - https://stackoverflow.com/questions/38770152/firefox-blurs-when-changing-the-input-type And there appears to be multiple related issues on bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=981248 https://bugzilla.mozilla.org/show_bug.cgi?id=1012818
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
I had this WIP patch around, which simplifies a bunch of stuff and should fix this as well, but needs a bit more work. I didn't want to lose track of it.
Updated•6 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 10•5 years ago
|
||
See bug 1547409. Migrating webcompat priority whiteboard tags to project flags.
Updated•5 years ago
|
Comment 11•5 years ago
|
||
I could not find a test like this already in the web platform tests (or the Blink or WebKit test suites), so here is a testharness.js WPT (like uievents/click/auxclick_event.html
) which should do the trick.
Assignee | ||
Comment 12•5 years ago
|
||
I had a patch to stop using the hacky anonymous text control for number inputs somewhere in bugzilla, I should probably rescue that patch.
Assignee | ||
Comment 13•5 years ago
|
||
Oh, it was attached to this bug. :-)
I'll try to rebase it.
Assignee | ||
Comment 14•5 years ago
|
||
I have https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b09ae8ee993fb9c9c055118153ef8b32e85b826, which kinda works. I just pushed it to try to see where it stands.
Assignee | ||
Comment 16•5 years ago
|
||
So from the try run:
- Needs some a11y fixes (this should be easy-ish).
- I broke some font inflation stuff looks like, and need to check the placeholder / preview stuff. I probably need to tweak some reflow bits or some CSS there to make layout correct and consistent.
- There's an assert that needs investigation. Potentially related to the above.
- I broke localization of the number but that (a) it is untested (b) it is bogus and (c) no other browser does it.
So all in all seems a bit of work, but probably workable.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 17•5 years ago
|
||
This is also breaking Sunlife's page on Android Firefox browsers, because it causes the on-screen keyboard to never be brought up and users cannot type in their age/etc.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 18•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 19•4 years ago
|
||
Comment on attachment 9115915 [details]
Bug 981248 - Rewrite <input type=number> to avoid an anonymous input. r=masayuki!,jfkthame!,surkov!,jwatt!,ntim!
Mats, Jonathan, interested in your thoughts on this.
I still need to fix a few things, mainly:
- Text has the wrong baseline (though input element uses the right one).
::placeholder
/preview-div
stuff needs to be fixed to be out of flow and positioned with CSS or something, instead of the current thing we do hiding them during the display list...
I still haven't decided whether I want the wrapper div everywhere. It may be worth simplifying the setup, plus it can be the containing block for the placeholder / preview, and thus may save quite a bit of trouble. But I haven't investigated it much.
I also need to look into some of the DOM bits (value sanitization and such), but off-hand the amount of code that goes away looks pretty neat.
Assignee | ||
Comment 20•4 years ago
|
||
Also note, it does change the layout a bit. In particular the buttons now appear inside the <input>
background. But that's a feature IMO, in my experience number inputs at least on Linux look terrible, and with my patch they look nicer to me at least.
Comment 21•4 years ago
|
||
Comment on attachment 9115915 [details]
Bug 981248 - Rewrite <input type=number> to avoid an anonymous input. r=masayuki!,jfkthame!,surkov!,jwatt!,ntim!
Looks great so far! And yes, we might want to merge them eventually, but it's probably better to do that as a follow-up bug.
Updated•4 years ago
|
Assignee | ||
Comment 22•4 years ago
|
||
Masayuki, ni? for https://phabricator.services.mozilla.com/D57193#1760448.
Also, over-all feedback on the patch would be much appreciated, I do like that it removes a bunch of special code in layout and focus management.
Here's a try run with the latest bits of test fixes and such: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3cfaca2e461b8471c10cd0ee0ef8a2bbbfceab04
Apart of the input events that I'm asking you about I need to do some work on the a11y bits. But I've discussed that with the a11y folks and it looks straight-forward.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 23•4 years ago
•
|
||
The test fails with my <input type=number> rewrite because editor fails to
dispatch one input event caused by these mouse events.
The reason for this is that we schedule them from an input event, which fires
from here:
Note how at that point we still haven't decremented mPlaceholderBatch. That means
that other stuff that triggers input events from there will not dispatch events.
I think that's a bit unexpected, but it is a preexisting problem, and can't
happen for users because mouse events go through the event loop.
Updated•4 years ago
|
Assignee | ||
Comment 24•4 years ago
|
||
This one should be all green :-)
Assignee | ||
Comment 25•4 years ago
|
||
Comment on attachment 9115915 [details]
Bug 981248 - Rewrite <input type=number> to avoid an anonymous input. r=masayuki!,jfkthame!,surkov!,jwatt!,ntim!
Sent a review request instead, now that's green. Thanks y'all for your help!
Assignee | ||
Comment 26•4 years ago
|
||
Tom, do you want to send the WPT in a separate patch? I verified locally that it passes as expected, but it'd be great if you preserved authorship. I can roll it into the patch above otherwise, but...
A minor nit: It'd be amazing to drop a <link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=981248">
on the test too, so people looking at the test can get all the context :)
Comment 27•4 years ago
|
||
I'm actually alright with it being part of this patch. If you don't mind dropping that link into it, it should be more than enough of a trail for folks to follow back, if they really are curious who wrote it (or more importantly why). Is that alright with you?
Assignee | ||
Comment 28•4 years ago
|
||
Sure, why not.
Assignee | ||
Updated•4 years ago
|
Comment 29•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9fac9eb1c24c Fix test_input_number_mouse_events.html to not dispatch mouse events from input events. r=masayuki
Comment 30•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Updated•4 years ago
|
Comment 31•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1eace7bd28d9 Rewrite <input type=number> to avoid an anonymous input. r=masayuki,surkov,jwatt,ntim,jfkthame,smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/21167 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Assignee | ||
Comment 34•4 years ago
|
||
Comment 35•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7a96708cc8b7 Remove now-unneeded special-case in AutoFillDelegateTest.kt. r=snorp
Upstream PR was closed without merging
Comment 37•4 years ago
|
||
Backed out for multiple failures.
backout: https://hg.mozilla.org/integration/autoland/rev/89c1fd1db3b14b99560fa69ff2cb5dd9d2152950
-
https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=gv-junit&revision=1eace7bd28d9986cdd43f5ecb6393a4aa8e9bffc
failure log e.g.: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284860483&repo=autoland&lineNumber=1440 -
https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=wpt3&revision=1eace7bd28d9986cdd43f5ecb6393a4aa8e9bffc&selectedJob=284862847
failure log e.g.: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284862847&repo=autoland&lineNumber=48859
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 38•4 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/010bd182aff8 Rewrite <input type=number> to avoid an anonymous input. r=masayuki,surkov,jwatt,ntim,jfkthame,smaug
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Comment 40•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Upstream PR merged by moz-wptsync-bot
Updated•4 years ago
|
Description
•