'blur' on clicking padding area of 'number' input

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: yurko.sirko, Assigned: jessica)

Tracking

({testcase})

46 Branch
mozilla49
testcase
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8746592 [details]
sample.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36

Steps to reproduce:

I'm clicking on padding area of input[type='number']


Actual results:

blur event fires


Expected results:

blur should not fire on clicking padding area of input

Comment 1

3 years ago
Created attachment 8747006 [details]
sample.html (reporter's testcase)

Updated

3 years ago
Keywords: testcase
Jessica's been looking at input types recently. Maybe she can take a quick look here?
Flags: needinfo?(jjong)
Whiteboard: btpp-followup-2016-05-06
(In reply to Andrew Overholt [:overholt] from comment #2)
> Jessica's been looking at input types recently. Maybe she can take a quick
> look here?

Sure, let me take a look at this.
Assignee: nobody → jjong
Flags: needinfo?(jjong)
Hi Olli,

Do you mind giving me some hint on this?
I noticed that when clicking on the padding area, both focus and blur events are fired. I'm looking at EventStateManager::PostHandleEvent, but I am not sure whether it is the right place that handles this logic, cause it seems that only input=number has this issue.

Thanks.
Flags: needinfo?(bugs)
aha, I need to click right side of the arrow buttons...

So first question to answer is that where is the focus being moved to when blur is fired? what is focus event's originalTarget? (web page can't access that but C++ can and privileged js.)

And EventStateManager is probably wrong place to look at. 
You may need to tweak HTMLInputElement::IsHTMLFocusable so that we don't try to move focus while focus is already in the relevant input type=number element. 
It is also possible that tabindex attribute handling in nsNumberControlFrame.cpp isn't quite right.
Flags: needinfo?(bugs)

Comment 6

3 years ago
When I also faced this issue, I debugged that focus is being moved to the same element in blur event. You can get newly focused element as "document.activeElement". Please see this answer on how to do it: http://stackoverflow.com/a/11592974
Created attachment 8750208 [details] [diff] [review]
patch, v1.

When focusing on a number input control, the focus is redirected to the text control. So in 'CheckIfFocusable()', if the content to focus is a number control, we should redirect the check to it's anonymous text control, otherwise the number control gets blurred and focused again.
Comment on attachment 8750208 [details] [diff] [review]
patch, v1.

Review of attachment 8750208 [details] [diff] [review]:
-----------------------------------------------------------------

Olli, I have hacked nsFocusManager::GetRedirectedFocus() so that it returns the anon text control when it's a number input control, just like the other XUL elements.
Do you think it's a good approach for this bug? Thanks.
Attachment #8750208 - Flags: feedback?(bugs)
Whiteboard: btpp-followup-2016-05-06 → btpp-active
try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45793739325e&selectedJob=20555340

The proposed patch breaks 'dom/html/test/forms/test_input_typing_sanitization.html', it seems that when input type is changed from number to date, the first time .focus() is called, the content to focus is still a number control. Still looking into it...
Created attachment 8750710 [details] [diff] [review]
patch, v2.

This patch fixes the failure in 'dom/html/test/forms/test_input_typing_sanitization.html'.
Turns out that when input type is changed from 'number' to 'date', the HTMLInputElement's type is changed but the corresponding frame has not switched yet. This patch adds an extra check on HTMLInputElement's type in nsFocusManager.

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc5a649a0053&selectedJob=20611026
Attachment #8750208 - Attachment is obsolete: true
Attachment #8750208 - Flags: feedback?(bugs)
Attachment #8750710 - Flags: feedback?(bugs)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8750710 [details] [diff] [review]
patch, v2.

Review of attachment 8750710 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is ready for review, changing to r? :)
Attachment #8750710 - Flags: feedback?(bugs) → review?(bugs)
Comment on attachment 8750710 [details] [diff] [review]
patch, v2.

ok, I guess this works.
Can we get some tests for this? Perhaps something which 
checks using nsFocusManager that focus is in the inner element, but
document.activeElement still returns the <input type=number> ?
Attachment #8750710 - Flags: review?(bugs) → review+
Created attachment 8755342 [details] [diff] [review]
tests, v1.

Thanks Olli, here come the tests.

Will merge the patches once they are reviewed.
Attachment #8755342 - Flags: review?(bugs)
Attachment #8755342 - Flags: review?(bugs) → review+
Created attachment 8755726 [details] [diff] [review]
final patch. r=smaug
Attachment #8750710 - Attachment is obsolete: true
Attachment #8755342 - Attachment is obsolete: true
Attachment #8755726 - Flags: review+
try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f12da6706394

(compared with other try results, the intermittent failures in Win7 are not related to this bug)
Keywords: checkin-needed

Comment 17

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c1f4b7c2ebd
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1272474
You need to log in before you can comment on or make changes to this bug.