46088 – Style update error by autofocus processing in recalcStyle()

RESOLVED FIXED Bug 46088

Style update error by autofocus processing in recalcStyle()

https://bugs.webkit.org/show_bug.cgi?id=46088

Summary Style update error by autofocus processing in recalcStyle()

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	Details Formatted Diff Diff
Patch (5.47 KB, patch) 2011-02-18 09:22 PST, Bharathwaaj	no flags	Details Formatted Diff Diff
Fixed webkit coding style. (deleted) 2011-02-18 09:30 PST, Bharathwaaj	no flags	Details Formatted Diff Diff
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.