91058 2012-07-12 01:33:32 -0700 Modifying an attribute for which there is a selector in html.css does not cause a style recalc 2017-07-18 08:30:35 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 90931 1 falken webkit-unassigned ap edaena eric.carlson eric kling koivisto menard mitz pravind.2k4 rniwa oldest_to_newest 667511 0 falken 2012-07-12 01:33:32 -0700 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. 668139 1 ap 2012-07-12 13:17:44 -0700 Sounds like this could also affect audio element rendering when controls attribute is added or removed. 668162 2 eric 2012-07-12 13:41:35 -0700 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? 668669 3 152167 falken 2012-07-13 00:23:59 -0700 Created attachment 152167 selector testcase 668675 4 falken 2012-07-13 00:26:32 -0700 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. 673324 5 153211 falken 2012-07-19 02:12:27 -0700 Created attachment 153211 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. 694599 6 pravind.2k4 2012-08-14 00:29:50 -0700 Matt , is it possible to attach a test case as a html file n without the html.css changes ? 695665 7 falken 2012-08-14 19:16:14 -0700 (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) 152167 2012-07-13 00:23:59 -0700 2012-07-19 02:12:27 -0700 selector testcase selector-testcase.patch text/plain 2247 falken ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNzcy1hdHRyaWJ1dGUtc2VsZWN0 b3ItcmVjYWxjLXN0eWxlLWV4cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNz cy1hdHRyaWJ1dGUtc2VsZWN0b3ItcmVjYWxjLXN0eWxlLWV4cGVjdGVkLnR4dApuZXcgZmlsZSBt b2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi4wN2I4NjNkCi0tLSAvZGV2L251bGwKKysrIGIvTGF5 b3V0VGVzdHMvZmFzdC9jc3MvdWEtY3NzLWF0dHJpYnV0ZS1zZWxlY3Rvci1yZWNhbGMtc3R5bGUt ZXhwZWN0ZWQudHh0CkBAIC0wLDAgKzEsMTIgQEAKK1Rlc3RzIHRoYXQgY2hhbmdpbmcgYW4gYXR0 cmlidXRlIHdlIGhhdmUgYSBzZWxlY3RvciBmb3IgaW4gaHRtbC5jc3MgY2F1c2VzIHN0eWxlIHRv IHVwZGF0ZS4KKworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBt ZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKK1BBU1MgY29tcHV0ZWRT dHlsZS5nZXRQcm9wZXJ0eVZhbHVlKCdiYWNrZ3JvdW5kLWNvbG9yJykgaXMgJ3JnYigwLCAyNTUs IDApJworUEFTUyBjb21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tncm91bmQtY29s b3InKSBpcyAncmdiKDAsIDI1NSwgMCknCitQQVNTIHN1Y2Nlc3NmdWxseVBhcnNlZCBpcyB0cnVl CisKK1RFU1QgQ09NUExFVEUKK1RoaXMgc2hvdWxkIGhhdmUgYSBncmVlbiBiYWNrZ3JvdW5kLgor VGhpcyBzaG91bGQgYWxzbyBoYXZlIGEgZ3JlZW4gYmFja2dyb3VuZC4KZGlmZiAtLWdpdCBhL0xh eW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNzcy1hdHRyaWJ1dGUtc2VsZWN0b3ItcmVjYWxjLXN0eWxl Lmh0bWwgYi9MYXlvdXRUZXN0cy9mYXN0L2Nzcy91YS1jc3MtYXR0cmlidXRlLXNlbGVjdG9yLXJl Y2FsYy1zdHlsZS5odG1sCm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAuLjg0NGM0 NWUKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2Nzcy91YS1jc3MtYXR0cmli dXRlLXNlbGVjdG9yLXJlY2FsYy1zdHlsZS5odG1sCkBAIC0wLDAgKzEsMjMgQEAKKzwhRE9DVFlQ RSBodG1sPgorPGh0bWw+Cis8aGVhZD4KKzxzY3JpcHQgc3JjPSIuLi9qcy9yZXNvdXJjZXMvanMt dGVzdC1wcmUuanMiPjwvc2NyaXB0PgorPC9oZWFkPgorPGJvZHk+Cis8c3BhbiBpZD0iZWwiPlRo aXMgc2hvdWxkIGhhdmUgYSBncmVlbiBiYWNrZ3JvdW5kLjwvc3Bhbj48YnI+Cis8c3BhbiBpZD0i ZWwyIiBvcGVuPlRoaXMgc2hvdWxkIGFsc28gaGF2ZSBhIGdyZWVuIGJhY2tncm91bmQuPC9zcGFu PgorPHNjcmlwdD4KK2Rlc2NyaXB0aW9uKCJUZXN0cyB0aGF0IGNoYW5naW5nIGFuIGF0dHJpYnV0 ZSB3ZSBoYXZlIGEgc2VsZWN0b3IgZm9yIGluIGh0bWwuY3NzIGNhdXNlcyBzdHlsZSB0byB1cGRh dGUuIik7CisKK2VsID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ2VsJyk7CitlbC5zZXRBdHRy aWJ1dGUoJ29wZW4nLCAnJykKK2NvbXB1dGVkU3R5bGUgPSB3aW5kb3cuZ2V0Q29tcHV0ZWRTdHls ZShlbCwgbnVsbCk7CitzaG91bGRCZSgiY29tcHV0ZWRTdHlsZS5nZXRQcm9wZXJ0eVZhbHVlKCdi YWNrZ3JvdW5kLWNvbG9yJykiLCAiJ3JnYigwLCAyNTUsIDApJyIpOworCitlbCA9IGRvY3VtZW50 LmdldEVsZW1lbnRCeUlkKCdlbDInKTsKK2NvbXB1dGVkU3R5bGUgPSB3aW5kb3cuZ2V0Q29tcHV0 ZWRTdHlsZShlbCwgbnVsbCk7CitzaG91bGRCZSgiY29tcHV0ZWRTdHlsZS5nZXRQcm9wZXJ0eVZh bHVlKCdiYWNrZ3JvdW5kLWNvbG9yJykiLCAiJ3JnYigwLCAyNTUsIDApJyIpOworPC9zY3JpcHQ+ Cis8c2NyaXB0IHNyYz0iLi4vanMvcmVzb3VyY2VzL2pzLXRlc3QtcG9zdC5qcyI+PC9zY3JpcHQ+ Cis8L2JvZHk+Cis8L2h0bWw+CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9jc3MvaHRtbC5j c3MgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvaHRtbC5jc3MKaW5kZXggMTc2OWQ0OS4uMzcxMDAwYyAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvY3NzL2h0bWwuY3NzCisrKyBiL1NvdXJjZS9XZWJD b3JlL2Nzcy9odG1sLmNzcwpAQCAtMTAyMyw1ICsxMDIzLDkgQEAgYmRvIHsKICAgICBib3JkZXIt d2lkdGg6IDBweDsKIH0KIAorc3BhbltvcGVuXSB7CisgIGJhY2tncm91bmQtY29sb3I6IHJnYigw LDI1NSwwKQorfQorCiAvKiBub3NjcmlwdCBpcyBoYW5kbGVkIGludGVybmFsbHksIGFzIGl0IGRl cGVuZHMgb24gc2V0dGluZ3MuICovCiAK 153211 2012-07-19 02:12:27 -0700 2012-07-19 02:12:27 -0700 better testcase that uses <pre> and <code> instead of <span> selector-testcase2.patch text/plain 3076 falken ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNzcy1hdHRyaWJ1dGUtc2VsZWN0 b3ItcmVjYWxjLXN0eWxlLWV4cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNz cy1hdHRyaWJ1dGUtc2VsZWN0b3ItcmVjYWxjLXN0eWxlLWV4cGVjdGVkLnR4dApuZXcgZmlsZSBt b2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi5hODAwZTRlCi0tLSAvZGV2L251bGwKKysrIGIvTGF5 b3V0VGVzdHMvZmFzdC9jc3MvdWEtY3NzLWF0dHJpYnV0ZS1zZWxlY3Rvci1yZWNhbGMtc3R5bGUt ZXhwZWN0ZWQudHh0CkBAIC0wLDAgKzEsMTggQEAKK1Rlc3RzIHRoYXQgY2hhbmdpbmcgYW4gYXR0 cmlidXRlIHdlIGhhdmUgYSBzZWxlY3RvciBmb3IgaW4gaHRtbC5jc3MgY2F1c2VzIHN0eWxlIHRv IHVwZGF0ZS4KKworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBt ZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKK1BBU1MgY29tcHV0ZWRT dHlsZS5nZXRQcm9wZXJ0eVZhbHVlKCdiYWNrZ3JvdW5kLWNvbG9yJykgaXMgJ3JnYigwLCAyNTUs IDApJworUEFTUyBjb21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tncm91bmQtY29s b3InKSBpcyAncmdiKDAsIDI1NSwgMCknCitQQVNTIGNvbXB1dGVkU3R5bGUuZ2V0UHJvcGVydHlW YWx1ZSgnYmFja2dyb3VuZC1jb2xvcicpIGlzICdyZ2IoMCwgMCwgMjU1KScKK1BBU1MgY29tcHV0 ZWRTdHlsZS5nZXRQcm9wZXJ0eVZhbHVlKCdiYWNrZ3JvdW5kLWNvbG9yJykgaXMgJ3JnYigwLCAw LCAyNTUpJworUEFTUyBzdWNjZXNzZnVsbHlQYXJzZWQgaXMgdHJ1ZQorCitURVNUIENPTVBMRVRF CitUaGlzIHNob3VsZCBoYXZlIGEgZ3JlZW4gYmFja2dyb3VuZC4KKworVGhpcyBzaG91bGQgYWxz byBoYXZlIGEgZ3JlZW4gYmFja2dyb3VuZC4KKworVGhpcyBzaG91bGQgaGF2ZSBhIGJsdWUgYmFj a2dyb3VuZC4KK1RoaXMgc2hvdWxkIGFsc28gaGF2ZSBhIGJsdWUgYmFja2dyb3VuZC4KZGlmZiAt LWdpdCBhL0xheW91dFRlc3RzL2Zhc3QvY3NzL3VhLWNzcy1hdHRyaWJ1dGUtc2VsZWN0b3ItcmVj YWxjLXN0eWxlLmh0bWwgYi9MYXlvdXRUZXN0cy9mYXN0L2Nzcy91YS1jc3MtYXR0cmlidXRlLXNl bGVjdG9yLXJlY2FsYy1zdHlsZS5odG1sCm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAw MDAuLjllNWEyNWMKLS0tIC9kZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2Nzcy91YS1j c3MtYXR0cmlidXRlLXNlbGVjdG9yLXJlY2FsYy1zdHlsZS5odG1sCkBAIC0wLDAgKzEsMzggQEAK KzwhRE9DVFlQRSBodG1sPgorPGh0bWw+Cis8aGVhZD4KKzxzY3JpcHQgc3JjPSIuLi9qcy9yZXNv dXJjZXMvanMtdGVzdC1wcmUuanMiPjwvc2NyaXB0PgorPHN0eWxlIHR5cGU9InRleHQvY3NzIj4K K2NvZGVbb3Blbl0geyBiYWNrZ3JvdW5kLWNvbG9yOiBibHVlIH0KKzwvc3R5bGU+Cis8L2hlYWQ+ Cis8Ym9keT4KKzxwcmUgaWQ9ImVsIj5UaGlzIHNob3VsZCBoYXZlIGEgZ3JlZW4gYmFja2dyb3Vu ZC48L3ByZT48YnI+Cis8cHJlIGlkPSJlbDIiIG9wZW4+VGhpcyBzaG91bGQgYWxzbyBoYXZlIGEg Z3JlZW4gYmFja2dyb3VuZC48L3ByZT48YnI+Cis8Y29kZSBpZD0iZWwzIj5UaGlzIHNob3VsZCBo YXZlIGEgYmx1ZSBiYWNrZ3JvdW5kLjwvY29kZT48YnI+Cis8Y29kZSBpZD0iZWw0IiBvcGVuPlRo aXMgc2hvdWxkIGFsc28gaGF2ZSBhIGJsdWUgYmFja2dyb3VuZC48L2NvZGU+CisKKzxzY3JpcHQ+ CitkZXNjcmlwdGlvbigiVGVzdHMgdGhhdCBjaGFuZ2luZyBhbiBhdHRyaWJ1dGUgd2UgaGF2ZSBh IHNlbGVjdG9yIGZvciBpbiBodG1sLmNzcyBjYXVzZXMgc3R5bGUgdG8gdXBkYXRlLiIpOworCitl bCA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCdlbCcpOworZWwuc2V0QXR0cmlidXRlKCdvcGVu JywgJycpOworY29tcHV0ZWRTdHlsZSA9IHdpbmRvdy5nZXRDb21wdXRlZFN0eWxlKGVsLCBudWxs KTsKK3Nob3VsZEJlKCJjb21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tncm91bmQt Y29sb3InKSIsICIncmdiKDAsIDI1NSwgMCknIik7CisKK2VsID0gZG9jdW1lbnQuZ2V0RWxlbWVu dEJ5SWQoJ2VsMicpOworY29tcHV0ZWRTdHlsZSA9IHdpbmRvdy5nZXRDb21wdXRlZFN0eWxlKGVs LCBudWxsKTsKK3Nob3VsZEJlKCJjb21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tn cm91bmQtY29sb3InKSIsICIncmdiKDAsIDI1NSwgMCknIik7CisKK2VsID0gZG9jdW1lbnQuZ2V0 RWxlbWVudEJ5SWQoJ2VsMycpOworZWwuc2V0QXR0cmlidXRlKCdvcGVuJywgJycpOworY29tcHV0 ZWRTdHlsZSA9IHdpbmRvdy5nZXRDb21wdXRlZFN0eWxlKGVsLCBudWxsKTsKK3Nob3VsZEJlKCJj b21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tncm91bmQtY29sb3InKSIsICIncmdi KDAsIDAsIDI1NSknIik7CisKK2VsID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ2VsNCcpOwor Y29tcHV0ZWRTdHlsZSA9IHdpbmRvdy5nZXRDb21wdXRlZFN0eWxlKGVsLCBudWxsKTsKK3Nob3Vs ZEJlKCJjb21wdXRlZFN0eWxlLmdldFByb3BlcnR5VmFsdWUoJ2JhY2tncm91bmQtY29sb3InKSIs ICIncmdiKDAsIDAsIDI1NSknIik7Cis8L3NjcmlwdD4KKzxzY3JpcHQgc3JjPSIuLi9qcy9yZXNv dXJjZXMvanMtdGVzdC1wb3N0LmpzIj48L3NjcmlwdD4KKzwvYm9keT4KKzwvaHRtbD4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9odG1sLmNzcyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9o dG1sLmNzcwppbmRleCA1YWFiMDZmLi5hMDJjZTg2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9jc3MvaHRtbC5jc3MKKysrIGIvU291cmNlL1dlYkNvcmUvY3NzL2h0bWwuY3NzCkBAIC0xMDI3 LDUgKzEwMjcsOSBAQCBiZG8gewogICAgIGJvcmRlci13aWR0aDogMHB4OwogfQogCitwcmVbb3Bl bl0geworICBiYWNrZ3JvdW5kLWNvbG9yOiByZ2IoMCwyNTUsMCkKK30KKwogLyogbm9zY3JpcHQg aXMgaGFuZGxlZCBpbnRlcm5hbGx5LCBhcyBpdCBkZXBlbmRzIG9uIHNldHRpbmdzLiAqLwogCg==