RESOLVED INVALID 68876
Input element created at later stage(may be after page load) should not be auto focused.
https://bugs.webkit.org/show_bug.cgi?id=68876
Summary Input element created at later stage(may be after page load) should not be au...
Rakesh
Reported 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.
Attachments
Test case (651 bytes, text/html)
2011-09-27 00:38 PDT, Rakesh
no flags
Proposed patch (3.69 KB, patch)
2011-09-28 06:22 PDT, Rakesh
no flags
Updated patch (6.33 KB, patch)
2011-10-03 00:20 PDT, Rakesh
eric: review-
Rakesh
Comment 1 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.
Kent Tamura
Comment 2 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.
Rakesh
Comment 3 2011-09-28 06:22:55 PDT
Created attachment 109016 [details] Proposed patch
Rakesh
Comment 4 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.
Rakesh
Comment 5 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.
WebKit Review Bot
Comment 6 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
Kent Tamura
Comment 7 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?
Rakesh
Comment 8 2011-10-03 00:20:36 PDT
Created attachment 109447 [details] Updated patch
Rakesh
Comment 9 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).
Kent Tamura
Comment 10 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.
Rakesh
Comment 11 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.
Eric Seidel (no email)
Comment 12 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...
Ojan Vafai
Comment 13 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.
Ojan Vafai
Comment 14 2012-03-30 12:38:08 PDT
In fact, the other bug is already filed: https://bugs.webkit.org/show_bug.cgi?id=31032
Note You need to log in before you can comment on or make changes to this bug.