Bug 68876 - Input element created at later stage(may be after page load) should not be auto focused.
Summary: Input element created at later stage(may be after page load) should not be au...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-27 00:38 PDT by Rakesh
Modified: 2012-03-30 12:38 PDT (History)
4 users (show)

See Also:


Attachments
Test case (651 bytes, text/html)
2011-09-27 00:38 PDT, Rakesh
no flags Details
Proposed patch (3.69 KB, patch)
2011-09-28 06:22 PDT, Rakesh
no flags Details | Formatted Diff | Diff
Updated patch (6.33 KB, patch)
2011-10-03 00:20 PDT, Rakesh
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rakesh 2011-09-27 00:38:28 PDT
Created attachment 108799 [details]
Test case

Steps to reproduce:

1. Run attached html file.
2. Click on the "click me" button.
3. The new input element should get created on button click.

Expected:
The new input element should not be focused.

Current behavior:
The new input element is being focused.

Others:
Firefox does not focus the newly created element.
Comment 1 Rakesh 2011-09-27 03:55:27 PDT
I think Document::setIgnoreAutofocus needs to be called from some place, may be  just before firing 'onload' event as we are sure if there was any autofocus node, then it might have been focused and we don't want any future elements with autofocus to be focused.

Please let me know your thoughts on this.
Thanks.
Comment 2 Kent Tamura 2011-09-27 17:52:36 PDT
I haven't seen requests to add other autofocus-canceling events. So, I don't think we need to do so.

If other browsers cancels autofocus by other events, it's ok to follow them in WebKit.
Comment 3 Rakesh 2011-09-28 06:22:55 PDT
Created attachment 109016 [details]
Proposed patch
Comment 4 Rakesh 2011-09-28 06:27:12 PDT
(In reply to comment #2)
> I haven't seen requests to add other autofocus-canceling events. So, I don't think we need to do so.
> 
> If other browsers cancels autofocus by other events, it's ok to follow them in WebKit.

Yes, firefox does cancel the autofocus and it fulfills
point 8 of http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#attr-fe-autofocus.
Comment 5 Rakesh 2011-09-28 06:30:35 PDT
(In reply to comment #3)
> Created an attachment (id=109016) [details]
> Proposed patch

This patch fails two tests:
1. fast/forms/autofocus-opera-004.html
2. fast/forms/autofocus-opera-005.html

which are expected with this change as autofocus is ignored after page load complete.
Comment 6 WebKit Review Bot 2011-09-28 06:48:39 PDT
Comment on attachment 109016 [details]
Proposed patch

Attachment 109016 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9878656

New failing tests:
fast/forms/autofocus-opera-004.html
fast/forms/autofocus-opera-005.html
Comment 7 Kent Tamura 2011-10-02 03:45:03 PDT
Comment on attachment 109016 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109016&action=review

r- because the patch wil break existing tests.
We have to change the patch behavior or change the existing tests in this case.

> Source/WebCore/dom/Document.cpp:2165
> +    // Ignore any future Autofocus request from now
> +    setIgnoreAutofocus();
> +

What are the behaviors of IE10, Firefox, and Opera on this?
Comment 8 Rakesh 2011-10-03 00:20:36 PDT
Created attachment 109447 [details]
Updated patch
Comment 9 Rakesh 2011-10-03 00:25:25 PDT
(In reply to comment #7)
> (From update of attachment 109016 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109016&action=review
> 
> r- because the patch wil break existing tests.
> We have to change the patch behavior or change the existing tests in this case.
> 

Changed the existing tests.

> > Source/WebCore/dom/Document.cpp:2165
> > +    // Ignore any future Autofocus request from now
> > +    setIgnoreAutofocus();
> > +
> 
> What are the behaviors of IE10, Firefox, and Opera on this?

Firefox and Opera does not auto focus elements which are added at later stages,
could not test on IE10(don't have an windows 7 machine).
Comment 10 Kent Tamura 2011-10-03 03:56:52 PDT
(In reply to comment #9)
> (In reply to comment #7)
> > (From update of attachment 109016 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=109016&action=review
> > 
> > r- because the patch wil break existing tests.
> > We have to change the patch behavior or change the existing tests in this case.
> > 
> 
> Changed the existing tests.
> 
> > > Source/WebCore/dom/Document.cpp:2165
> > > +    // Ignore any future Autofocus request from now
> > > +    setIgnoreAutofocus();
> > > +
> > 
> > What are the behaviors of IE10, Firefox, and Opera on this?
> 
> Firefox and Opera does not auto focus elements which are added at later stages,
> could not test on IE10(don't have an windows 7 machine).

My investigation:
Firefox 6 and 7 Mac: Failed with autofocus-opera-004, 005, and 006.
Opera 11.51 Mac: Failed with autofocus-opera-008
IE10 on Windows 8: Failed with autofocus-opera-006 and 008.

IMO, we should do nothing at this moment. Autofocusing on an element with autofocus attribute is what a web page author wants to do even after the page loading has been completed, and the standard allows it.
Comment 11 Rakesh 2011-10-03 06:08:22 PDT
(In reply to comment #10)
> 
> My investigation:
> Firefox 6 and 7 Mac: Failed with autofocus-opera-004, 005, and 006.
> Opera 11.51 Mac: Failed with autofocus-opera-008
> IE10 on Windows 8: Failed with autofocus-opera-006 and 008.
> 
> IMO, we should do nothing at this moment. Autofocusing on an element with autofocus attribute is what a web page author wants to do even after the page loading has been completed, and the standard allows it.

True, but if focus is already set at some other node then I think it would be better to ignore the 'autofocus', that is what Opera or Firefox does if you run the attached test case (focus is on "Click me" button and not on the newly created input element).

What I observed in Opera and firefox is when I click on button, autofocus is not honored, also better test I tried :  

function onLoad()
{
  setTimeout(addInputElement, 2000);
}
and I start typing something in existing input element, the focus does not move to newly created element where as in webkit it moves to new element.
Comment 12 Eric Seidel (no email) 2011-12-19 14:00:43 PST
Comment on attachment 109447 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109447&action=review

> LayoutTests/fast/forms/autofocus-add-autofocus-element-expected.txt:5
> +This input element should not have active state: 
> +
> +
> +PASS document.activeElement is document.getElementById("input")

These lines seem incongruous.

> LayoutTests/fast/forms/autofocus-opera-004-expected.txt:1
> -The form control should have a green background: 
> +The form control should have a red background: 

Red is normally an indication of failure in tests...
Comment 13 Ojan Vafai 2012-03-30 12:35:47 PDT
Relevant Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=662496.

I don't see any reason to make autofocus conditional on page load. That's a totally arbitrary time. Might as well pick a fixed number of seconds from starting load. 

Item 8 of http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#attr-fe-autofocus says nothing about page load. It says that if the *user* indicated in some way that they want focus elsewhere the UA can ignore this attribute if they want to.

I'm going to close this bug. A new bug for ignoring autofocus if the user has started typing in an input might be worth considering. But I think we should only do that if this is a problem web pages are hitting in practice, which seems unlikely to me.
Comment 14 Ojan Vafai 2012-03-30 12:38:08 PDT
In fact, the other bug is already filed: https://bugs.webkit.org/show_bug.cgi?id=31032