Bug 46088

Summary: Style update error by autofocus processing in recalcStyle()
Product: WebKit Reporter: Sidney San Martín <s+webkit>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: abarth, adele, ap, bharathwaaj.s, bolinfest, cfleizach, dglazkov, johan, martin, michelangelo, mihaip, mitz, musicisair, paulirish, tkent, webkit.review.bot
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://dev.deeptechinc.com/sidney/share/webkit_autofocus_autofill/
Attachments:
Description Flags
Self-contained demonstration of the bug.
none
Patch
none
Patch
none
Fixed webkit coding style. none

Sidney San Martín
Reported 2010-09-20 07:46:36 PDT
This bug is reproducible under a strange set of conditions. It's in the Safari 5.0.2 release. If a form has `autofocus` on an input, the focus rings do not display on that page and autofill does not happen as long as: - The page has an external style sheet (`<link>` or `@import`) - You did not visit the page directly from a link (the bug *does* appear after a reload or navigating to the URL manually) - You did not visit the page using the back button. The bug goes away the first time you mouse down on a button or link. See the URL for a demo.
Attachments
Self-contained demonstration of the bug. (846 bytes, text/html)
2010-10-29 05:08 PDT, Martin Troels Eberhardt 		no flags
Details
Patch (6.32 KB, patch)
2011-01-20 02:39 PST, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch (5.47 KB, patch)
2011-02-18 09:22 PST, Bharathwaaj 		no flags
DetailsFormatted DiffDiff
Fixed webkit coding style. (deleted)
2011-02-18 09:30 PST, Bharathwaaj 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sidney San Martín
Comment 1 2010-09-20 07:48:09 PDT
Forgot to mention: because the bug doesn't reproduce on a page visited from a link, reload the demo page to see it in action.
Martin Troels Eberhardt
Comment 2 2010-10-29 05:08:58 PDT
Created attachment 72317 [details] Self-contained demonstration of the bug. I think I have come across the same bug, but in another form. The stylesheet does not need to be external. Not only does the focus-ring not show, but hidden elements, that you try to show using javascript, doesn't get displayed, before you hover over an element that has a :hover pseudoclass style. I have made an example that is self-contained and doesn't use an external stylesheet.
Mihai Parparita
Comment 3 2011-01-13 17:37:44 PST
This was also reported as http://crbug.com/69607, and has a possibly better test-case at http://bolinfest.com/webkit/chrome8bug.html. Also cc-ing the author of autofocus support (from http://trac.webkit.org/changeset/34626).
Kent Tamura
Comment 4 2011-01-19 00:50:02 PST
http://crbug.com/65695 also looks the same issue. Style recalc or repainting is blocked?
Kent Tamura
Comment 5 2011-01-19 20:30:24 PST
(In reply to comment #4) > Style recalc or repainting is blocked? I found :focus style is not applied to the element with autofocus. Document::recalcStyle() is called after setFocusedNode(), but CSSStyleSelector::SelectorChecker::checkOneSelector() for the element is not called.
Kent Tamura
Comment 6 2011-01-19 22:26:00 PST
ok, probably I understand. When an external stylesheet is linked or a sub-tree with autofocus is shown, an input element with autofocus is attached in Element::recalcStyle(). HTMLFormControlElement::attach() calls focus(), and focus() tries to call recalcStyle() to apply :focus style, but the latter recalStyle() is skipped because we are already in recalcStyle().
Kent Tamura
Comment 7 2011-01-20 02:39:22 PST
Created attachment 79569 [details] Patch
Dimitri Glazkov (Google)
Comment 8 2011-01-25 18:39:30 PST
It seems wrong to need an extra message loop cycle to get this right.
David Murdoch
Comment 9 2011-02-03 11:25:54 PST
Here is another test case for this bug: http://test.vervestudios.co/test.html To reproduced the bug in Webkit: 1. Focus your browser's address bar and press enter. 2. Hover over any button to see that its background-color will not change. 3. Try clicking a button. Hm, the style doesn't change. 4. Press F5 5. Now try hovering over a button. Viola! It works. so does clicking it. So, what triggers this bug? - An external stylesheet (even if it is empty). - An input[type=text] (or just <input>) with the [autofocus] attribute - A button - Both the text-input and button need to be inside the same containing element (and <body> won't trigger the bug) Some interesting stuff: - The buttons' onclick, onmouseover, etc. events are fired even when bug is present. - If you use javascript to edit a className the styles won't update. - In certain circumstances, if you hover over an anchor link the bug disappears. I haven't been able to reproduce this in a reduced test-case. (corresponding bug ion webkit's bug-tracker says that the anchor needs a :hover style...I haven't tried it though)
Bharathwaaj
Comment 10 2011-02-18 09:22:29 PST
Created attachment 82969 [details] Patch
WebKit Review Bot
Comment 11 2011-02-18 09:25:17 PST
Attachment 82969 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/html/HTMLFormControlElement.cpp:125: Declaration has space between type name and * in HTMLFormControlElement *control [whitespace/declaration] [3] Source/WebCore/html/HTMLFormControlElement.cpp:147: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Bharathwaaj
Comment 12 2011-02-18 09:30:48 PST
Created attachment 82970 [details] Fixed webkit coding style.
Bharathwaaj
Comment 13 2011-02-18 19:19:14 PST
I don't know why it is failing to apply in efl-ews bot.
Adam Barth
Comment 14 2011-02-18 19:22:09 PST
fatal: Unable to create '/mnt/eflews/git/webkit/.git/index.lock': File exists. Looks like the efl ews bot has a git lock problem. I wouldn't worry about it.
Kent Tamura
Comment 15 2011-02-18 19:40:10 PST
Comment on attachment 82970 [details] Fixed webkit coding style. queuePostAttachCallback() looks better. However I shouldn't set review+ because the patch is based on my code :-D
Eric Seidel (no email)
Comment 16 2011-02-24 03:12:22 PST
Comment on attachment 82970 [details] Fixed webkit coding style. View in context: https://bugs.webkit.org/attachment.cgi?id=82970&action=review > Source/WebCore/html/HTMLFormControlElement.cpp:146 > + queuePostAttachCallback(updateAutoFocusCallback, this); post attach callbacks are the work of the devil! :(
Bharathwaaj
Comment 17 2011-02-24 06:30:39 PST
(In reply to comment #16) > (From update of attachment 82970 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82970&action=review > > > Source/WebCore/html/HTMLFormControlElement.cpp:146 > > + queuePostAttachCallback(updateAutoFocusCallback, this); > > post attach callbacks are the work of the devil! :( Thank you for the comment. I will come up with another solution. But can you please help me understand why it is not advisable to use queuePostAttachCallback? I would like to learn so that I don't repeat this in the future.
Kent Tamura
Comment 18 2011-03-09 16:06:50 PST
*** Bug 55063 has been marked as a duplicate of this bug. ***
Kent Tamura
Comment 19 2011-03-30 21:38:33 PDT
*** Bug 57431 has been marked as a duplicate of this bug. ***
Kent Tamura
Comment 20 2011-03-30 21:39:52 PDT
According to Bug 57431, this autofocus problem causes a crash. Raise to P1.
Kent Tamura
Comment 21 2011-05-23 00:07:04 PDT
This was fixed by another similar patch in another bug. https://trac.webkit.org/changeset/86976
Sidney San Martín
Comment 22 2011-06-02 13:31:51 PDT
"Self-contained demonstration of the bug." still does not work properly. Is that the same bug or a different one?
Kent Tamura
Comment 23 2011-06-02 18:28:05 PDT
(In reply to comment #22) > "Self-contained demonstration of the bug." still does not work properly. Is that the same bug or a different one? I worked on Google Chrome 13.0.782.1 (WebKit r87771).
Sidney San Martín
Comment 24 2011-06-02 19:54:41 PDT
(In reply to comment #23) > (In reply to comment #22) > > "Self-contained demonstration of the bug." still does not work properly. Is that the same bug or a different one? > > I worked on Google Chrome 13.0.782.1 (WebKit r87771). Oh. Brain fart, I was in Chrome 11 (r86024). Looking good here too! Thanks :) .
Note You need to log in before you can comment on or make changes to this bug.