Bug 46088 - Style update error by autofocus processing in recalcStyle()
Summary: Style update error by autofocus processing in recalcStyle()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Major
Assignee: Nobody
URL: http://dev.deeptechinc.com/sidney/sha...
Keywords:
: 55063 57431 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-20 07:46 PDT by Sidney San Martín
Modified: 2011-06-02 19:54 PDT (History)
16 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Sidney San Martín 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.
Comment 1 Sidney San Martín 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.
Comment 2 Martin Troels Eberhardt 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.
Comment 3 Mihai Parparita 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).
Comment 4 Kent Tamura 2011-01-19 00:50:02 PST
http://crbug.com/65695 also looks the same issue.

Style recalc or repainting is blocked?
Comment 5 Kent Tamura 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.
Comment 6 Kent Tamura 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().
Comment 7 Kent Tamura 2011-01-20 02:39:22 PST
Created attachment 79569 [details]
Patch
Comment 8 Dimitri Glazkov (Google) 2011-01-25 18:39:30 PST
It seems wrong to need an extra message loop cycle to get this right.
Comment 9 David Murdoch 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)
Comment 10 Bharathwaaj 2011-02-18 09:22:29 PST
Created attachment 82969 [details]
Patch
Comment 11 WebKit Review Bot 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.
Comment 12 Bharathwaaj 2011-02-18 09:30:48 PST
Created attachment 82970 [details]
Fixed webkit coding style.
Comment 13 Bharathwaaj 2011-02-18 19:19:14 PST
I don't know why it is failing to apply in efl-ews bot.
Comment 14 Adam Barth 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.
Comment 15 Kent Tamura 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
Comment 16 Eric Seidel (no email) 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!  :(
Comment 17 Bharathwaaj 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.
Comment 18 Kent Tamura 2011-03-09 16:06:50 PST
*** Bug 55063 has been marked as a duplicate of this bug. ***
Comment 19 Kent Tamura 2011-03-30 21:38:33 PDT
*** Bug 57431 has been marked as a duplicate of this bug. ***
Comment 20 Kent Tamura 2011-03-30 21:39:52 PDT
According to Bug 57431, this autofocus problem causes a crash.
Raise to P1.
Comment 21 Kent Tamura 2011-05-23 00:07:04 PDT
This was fixed by another similar patch in another bug.
https://trac.webkit.org/changeset/86976
Comment 22 Sidney San Martín 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?
Comment 23 Kent Tamura 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).
Comment 24 Sidney San Martín 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 :) .