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

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Yuriy]

Attachment: 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

[Loic]

Attachment: sample.html (reporter's testcase)

Comment 2

[Andrew Overholt [:overholt]]

Jessica's been looking at input types recently. Maybe she can take a quick look here?

Comment 3

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 4

[Jessica Jong [:jessica] (inactive)]

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.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 6

[Sasha]

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

Comment 7

[Jessica Jong [:jessica] (inactive)]

Attachment: 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 8

[Jessica Jong [:jessica] (inactive)]

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.

Comment 9

[Jessica Jong [:jessica] (inactive)]

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...

Comment 10

[Jessica Jong [:jessica] (inactive)]

Attachment: 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

Comment 11

[Jessica Jong [:jessica] (inactive)]

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? :)

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

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> ?

Comment 13

[Jessica Jong [:jessica] (inactive)]

Attachment: tests, v1.

Thanks Olli, here come the tests.

Will merge the patches once they are reviewed.

Comment 14

[Jessica Jong [:jessica] (inactive)]

Attachment: final patch. r=smaug

Comment 15

[Jessica Jong [:jessica] (inactive)]

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)

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c1f4b7c2ebd

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0c1f4b7c2ebd