Bug 91058

Summary:

Modifying an attribute for which there is a selector in html.css does not cause a style recalc

Product:

WebKit

Reporter:

Matt Falkenhagen <falken>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

ap, edaena, eric.carlson, eric, kling, koivisto, menard, mitz, pravind.2k4, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

90931

Attachments:

Description	Flags
selector testcase	none
better testcase that uses <pre> and <code> instead of <span>	none

Matt Falkenhagen

Reported 2012-07-12 01:33:32 PDT

Modifying an attribute for which there is a selector in html.css does not initially cause a style recalc. But once a recalc is forced (e.g., by zooming in/out or opening debug console), modifying the attribute thereafter will cause the style recalc to happen. In practice, the bug does not occur in Safari, but it does occur when the layout tests are run. The bug occurs on Chromium in the browser itself and layout tests. I believe the problem is the Document's StyleResolver does not yet have m_features.attrsInRules populated, so the code in Element::attributeChanged doesn't cause a style recalc. Once the recalc is forced (as mentioned above), the attributeChanged will trigger the recalc. I discovered this in the patch for bug 90931 (https://bug-90931-attachments.webkit.org/attachment.cgi?id=151653). I tried to come up with a simpler test case, but it seems the relevant attributes in html.css already have special code to trigger a recalc. For example, iframe seamless has code in HTMLIFrameElement::parseAttribute: // If we're adding or removing the seamless attribute, we need to force the content document to recalculate its StyleResolver. if (contentDocument()) contentDocument()->styleResolverChanged(DeferRecalcStyle); I'll see if I can create a simpler test case apart from bug 90931 later.

Attachments
selector testcase (2.19 KB, patch) 2012-07-13 00:23 PDT, Matt Falkenhagen	no flags	Details Formatted Diff Diff
better testcase that uses <pre> and <code> instead of <span> (3.00 KB, patch) 2012-07-19 02:12 PDT, Matt Falkenhagen	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2012-07-12 13:17:44 PDT

Sounds like this could also affect audio element rendering when controls attribute is added or removed.

Eric Seidel (no email)

Comment 2 2012-07-12 13:41:35 PDT

I think the seamless example is a red-herring. :) switching into and out of seamless mode is a big change, causing you to need to recalculate your root style (which you normally don't do). I'm interested in this bug, but I'm not sure I understand what your describing yet. Do you have a test case?

Matt Falkenhagen

Comment 3 2012-07-13 00:23:59 PDT

Created attachment 152167 [details] selector testcase

Matt Falkenhagen

Comment 4 2012-07-13 00:26:32 PDT

I've attached a patch that illustrates what I'm seeing, i.e., the layout test doesn't have the desired results when run with run_webkit_tests or new-run-webkit-tests. I added to html.css to illustrate this as I couldn't get the bug to occur with existing attribute selectors. When this particular layout test is opened directly in the browser, as opposed to the layout test script, it seems like the bug doesn't occur. But with similar testcases, I get inconsistent results in Chrome: sometimes I see the bug and sometimes not. Oddly, the bug occurs even without dynamically changing the attribute, as seen in the second <span> of the layout test.

Matt Falkenhagen

Comment 5 2012-07-19 02:12:27 PDT

Created attachment 153211 [details] better testcase that uses <pre> and <code> instead of <span> I discovered the previous testcase was bad since <span> uses the simple default style in StyleResolver.cpp (elementCanUseSimpleDefaultStyle() returns true). I've modified the testcase to use <pre> and <code> instead. In this test case, an attribute selector pre[open] is defined in html.css and a code[open] one is defined in the layout test itself. When I run the test, <pre>'s does not take effect, but <code>'s does. I found that changing elementCanUseSimpleDefaultStyle() to always return false seems to fix the bug, but I'm not sure that is the real cause. If elementCanUseSimpleDefaultStyle() is not changed and instead the pre[open] CSS is simply added to the simpleUserAgentStyleSheet string, the bug still occurs. It seems it is some timing issue of when the stylesheet is read. It seems if I run a fresh instance of Chromium and immediately open the layout test, it fails, but if I then reload or open the test by clicking a link to it, it passes. Also, running other layout tests before this one causes it to pass; it only fails if it's the only test run.

Pravin D

Comment 6 2012-08-14 00:29:50 PDT

Matt , is it possible to attach a test case as a html file n without the html.css changes ?

Matt Falkenhagen

Comment 7 2012-08-14 19:16:14 PDT

(In reply to comment #6) > Matt , is it possible to attach a test case as a html file n without the html.css changes ? I wasn't able to get the bug to occur the existing selectors in html.css (see comment #4)

Note You need to log in before you can comment on or make changes to this bug.