Skip to content
Permalink
Browse files
Fix style sharing with the "type" and "readonly" attributes
https://bugs.webkit.org/show_bug.cgi?id=139283

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-12-05
Reviewed by Antti Koivisto.

Source/WebCore:

There are two bugs adressed with this patch:
1) The attributes "type" and "readonly" where only handled correctly
   for input elements. For everything else, they could incorrectly
   be ignored for style sharing.
2) The handling of attributes was incorrect for selector lists, leading
   to various bugs (incorrect style sharing in some cases, disabling
   style sharing on valid cases).

For [1], the problem was that attribute checking had been limited to
StyleResolver::canShareStyleWithControl(). That function is for handling
the special states of input element. For any other element, the attributes
were simply ignored.

For [2], there were a bunch of small problems. First, containsUncommonAttributeSelector()
was not recursive, which caused it to ignored any nested selector list. This used to be
correct but since we have advanced selectors we can no longer assumed selectors are not nested.

A second issue was that any attribute in a selector list was causing us to fall back
to the slow case. Now that we have the fast :matches(), we really don't want that.

The function containsUncommonAttributeSelector() was transformed into a recursive function
tracking where we are in the selector.

At the entry point, we start with the flag "startsOnRightmostElement" set to true. The flag is then
updated on the stack of each recursive call.

For example, "webkit > is:matches(freaking > awesome)". We evalute "is" with the flag to true, then recurse
into evaluating "freaking > awesome" with the flag still set to true. When we evalute ">", the flag
is set to false to evaluate any following selectors.
After evaluating "freaking > awesome", we go back to our previous stack frame, and the flag
is back to true and we can continue evaluating with the curren top level state.

From some logging, I discovered that the attribute handling is way too aggressive.
This is not a regression and I cannot fix that easily so I left a fixme.

Tests: fast/css/data-attribute-style-sharing-1.html
       fast/css/data-attribute-style-sharing-2.html
       fast/css/data-attribute-style-sharing-3.html
       fast/css/data-attribute-style-sharing-4.html
       fast/css/data-attribute-style-sharing-5.html
       fast/css/data-attribute-style-sharing-6.html
       fast/css/data-attribute-style-sharing-7.html
       fast/css/readonly-attribute-style-sharing-1.html
       fast/css/readonly-attribute-style-sharing-2.html
       fast/css/readonly-attribute-style-sharing-3.html
       fast/css/readonly-attribute-style-sharing-4.html
       fast/css/readonly-attribute-style-sharing-5.html
       fast/css/readonly-attribute-style-sharing-6.html
       fast/css/readonly-attribute-style-sharing-7.html
       fast/css/type-attribute-style-sharing-1.html
       fast/css/type-attribute-style-sharing-2.html
       fast/css/type-attribute-style-sharing-3.html
       fast/css/type-attribute-style-sharing-4.html
       fast/css/type-attribute-style-sharing-5.html
       fast/css/type-attribute-style-sharing-6.html
       fast/css/type-attribute-style-sharing-7.html

* css/RuleSet.cpp:
(WebCore::containsUncommonAttributeSelector):
(WebCore::RuleData::RuleData):
(WebCore::selectorListContainsAttributeSelector): Deleted.
* css/StyleResolver.cpp:
(WebCore::StyleResolver::canShareStyleWithControl):
(WebCore::StyleResolver::canShareStyleWithElement):

LayoutTests:

* fast/css/data-attribute-style-sharing-1-expected.html: Added.
* fast/css/data-attribute-style-sharing-1.html: Added.
* fast/css/data-attribute-style-sharing-2-expected.html: Added.
* fast/css/data-attribute-style-sharing-2.html: Added.
* fast/css/data-attribute-style-sharing-3-expected.html: Added.
* fast/css/data-attribute-style-sharing-3.html: Added.
* fast/css/data-attribute-style-sharing-4-expected.html: Added.
* fast/css/data-attribute-style-sharing-4.html: Added.
* fast/css/data-attribute-style-sharing-5-expected.html: Added.
* fast/css/data-attribute-style-sharing-5.html: Added.
* fast/css/data-attribute-style-sharing-6-expected.html: Added.
* fast/css/data-attribute-style-sharing-6.html: Added.
* fast/css/data-attribute-style-sharing-7-expected.html: Added.
* fast/css/data-attribute-style-sharing-7.html: Added.
* fast/css/readonly-attribute-style-sharing-1-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-1.html: Added.
* fast/css/readonly-attribute-style-sharing-2-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-2.html: Added.
* fast/css/readonly-attribute-style-sharing-3-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-3.html: Added.
* fast/css/readonly-attribute-style-sharing-4-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-4.html: Added.
* fast/css/readonly-attribute-style-sharing-5-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-5.html: Added.
* fast/css/readonly-attribute-style-sharing-6-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-6.html: Added.
* fast/css/readonly-attribute-style-sharing-7-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-7.html: Added.
* fast/css/type-attribute-style-sharing-1-expected.html: Added.
* fast/css/type-attribute-style-sharing-1.html: Added.
* fast/css/type-attribute-style-sharing-2-expected.html: Added.
* fast/css/type-attribute-style-sharing-2.html: Added.
* fast/css/type-attribute-style-sharing-3-expected.html: Added.
* fast/css/type-attribute-style-sharing-3.html: Added.
* fast/css/type-attribute-style-sharing-4-expected.html: Added.
* fast/css/type-attribute-style-sharing-4.html: Added.
* fast/css/type-attribute-style-sharing-5-expected.html: Added.
* fast/css/type-attribute-style-sharing-5.html: Added.
* fast/css/type-attribute-style-sharing-6-expected.html: Added.
* fast/css/type-attribute-style-sharing-6.html: Added.
* fast/css/type-attribute-style-sharing-7-expected.html: Added.
* fast/css/type-attribute-style-sharing-7.html: Added.


Canonical link: https://commits.webkit.org/157163@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@176864 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Benjamin Poulain authored and BenjaminPoulain committed Dec 5, 2014
1 parent 428d571 commit 1234b655a1e4294e32f23aaade5a6e8f1936cbee
Showing 46 changed files with 2,579 additions and 39 deletions.
@@ -1,3 +1,53 @@
2014-12-05 Benjamin Poulain <bpoulain@apple.com>

Fix style sharing with the "type" and "readonly" attributes
https://bugs.webkit.org/show_bug.cgi?id=139283

Reviewed by Antti Koivisto.

* fast/css/data-attribute-style-sharing-1-expected.html: Added.
* fast/css/data-attribute-style-sharing-1.html: Added.
* fast/css/data-attribute-style-sharing-2-expected.html: Added.
* fast/css/data-attribute-style-sharing-2.html: Added.
* fast/css/data-attribute-style-sharing-3-expected.html: Added.
* fast/css/data-attribute-style-sharing-3.html: Added.
* fast/css/data-attribute-style-sharing-4-expected.html: Added.
* fast/css/data-attribute-style-sharing-4.html: Added.
* fast/css/data-attribute-style-sharing-5-expected.html: Added.
* fast/css/data-attribute-style-sharing-5.html: Added.
* fast/css/data-attribute-style-sharing-6-expected.html: Added.
* fast/css/data-attribute-style-sharing-6.html: Added.
* fast/css/data-attribute-style-sharing-7-expected.html: Added.
* fast/css/data-attribute-style-sharing-7.html: Added.
* fast/css/readonly-attribute-style-sharing-1-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-1.html: Added.
* fast/css/readonly-attribute-style-sharing-2-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-2.html: Added.
* fast/css/readonly-attribute-style-sharing-3-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-3.html: Added.
* fast/css/readonly-attribute-style-sharing-4-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-4.html: Added.
* fast/css/readonly-attribute-style-sharing-5-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-5.html: Added.
* fast/css/readonly-attribute-style-sharing-6-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-6.html: Added.
* fast/css/readonly-attribute-style-sharing-7-expected.html: Added.
* fast/css/readonly-attribute-style-sharing-7.html: Added.
* fast/css/type-attribute-style-sharing-1-expected.html: Added.
* fast/css/type-attribute-style-sharing-1.html: Added.
* fast/css/type-attribute-style-sharing-2-expected.html: Added.
* fast/css/type-attribute-style-sharing-2.html: Added.
* fast/css/type-attribute-style-sharing-3-expected.html: Added.
* fast/css/type-attribute-style-sharing-3.html: Added.
* fast/css/type-attribute-style-sharing-4-expected.html: Added.
* fast/css/type-attribute-style-sharing-4.html: Added.
* fast/css/type-attribute-style-sharing-5-expected.html: Added.
* fast/css/type-attribute-style-sharing-5.html: Added.
* fast/css/type-attribute-style-sharing-6-expected.html: Added.
* fast/css/type-attribute-style-sharing-6.html: Added.
* fast/css/type-attribute-style-sharing-7-expected.html: Added.
* fast/css/type-attribute-style-sharing-7.html: Added.

2014-12-04 Dean Jackson <dino@apple.com>

css3/viewport-percentage-lengths tests are flakey on WK1 Mavericks Debug
@@ -0,0 +1,34 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div style="background-color: red;">data-webkit defined</div>
<div style="background-color: red;">data-webkit defined empty</div>
<div style="background-color: blue;">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span style="background-color: red;">data-webkit defined</span>
<span style="background-color: red;">data-webkit defined empty</span>
<span style="background-color: blue;">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input style="background-color: red;" value="data-webkit defined">
<input style="background-color: red;" value="data-webkit defined empty">
<input style="background-color: blue;" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,40 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
[data-webkit] {
background-color: red;
}
[data-webkit=foobar] {
background-color: blue;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div data-webkit>data-webkit defined</div>
<div data-webkit="">data-webkit defined empty</div>
<div data-webkit="foobar">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span data-webkit>data-webkit defined</span>
<span data-webkit="">data-webkit defined empty</span>
<span data-webkit="foobar">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input data-webkit value="data-webkit defined">
<input data-webkit="" value="data-webkit defined empty">
<input data-webkit="foobar" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,34 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div style="background-color: blue;">No data-webkit</div>
<div style="background-color: blue;">data-webkit defined</div>
<div style="background-color: blue;">data-webkit defined empty</div>
<div>data-webkit is foobar</div>
</div>
<div>
<span style="background-color: blue;">No data-webkit</span>
<span style="background-color: blue;">data-webkit defined</span>
<span style="background-color: blue;">data-webkit defined empty</span>
<span>data-webkit is foobar</span>
</div>
<div>
<input style="background-color: blue;" value="No data-webkit">
<input style="background-color: blue;" value="data-webkit defined">
<input style="background-color: blue;" value="data-webkit defined empty">
<input value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,40 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
div > :not([data-webkit]) {
background-color: red;
}
div > :not([data-webkit=foobar]) {
background-color: blue;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div data-webkit>data-webkit defined</div>
<div data-webkit="">data-webkit defined empty</div>
<div data-webkit="foobar">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span data-webkit>data-webkit defined</span>
<span data-webkit="">data-webkit defined empty</span>
<span data-webkit="foobar">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input data-webkit value="data-webkit defined">
<input data-webkit="" value="data-webkit defined empty">
<input data-webkit="foobar" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,34 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div style="background-color: red;">data-webkit defined</div>
<div style="background-color: red;">data-webkit defined empty</div>
<div style="background-color: blue;">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span style="background-color: red;">data-webkit defined</span>
<span style="background-color: red;">data-webkit defined empty</span>
<span style="background-color: blue;">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input style="background-color: red;" value="data-webkit defined">
<input style="background-color: red;" value="data-webkit defined empty">
<input style="background-color: blue;" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,40 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
:matches([data-webkit]) {
background-color: red;
}
:matches([data-webkit=foobar]) {
background-color: blue;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div data-webkit>data-webkit defined</div>
<div data-webkit="">data-webkit defined empty</div>
<div data-webkit="foobar">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span data-webkit>data-webkit defined</span>
<span data-webkit="">data-webkit defined empty</span>
<span data-webkit="foobar">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input data-webkit value="data-webkit defined">
<input data-webkit="" value="data-webkit defined empty">
<input data-webkit="foobar" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,34 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div style="background-color: red;">data-webkit defined</div>
<div style="background-color: red;">data-webkit defined empty</div>
<div style="background-color: blue;">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span style="background-color: red;">data-webkit defined</span>
<span style="background-color: red;">data-webkit defined empty</span>
<span style="background-color: blue;">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input style="background-color: red;" value="data-webkit defined">
<input style="background-color: red;" value="data-webkit defined empty">
<input style="background-color: blue;" value="data-webkit is foobar">
</div>
</body>
</html>
@@ -0,0 +1,40 @@
<!doctype html>
<html>
<head>
<style>
/* Pack them to fit everything in 800*600 */
div > * {
padding: 5px;
width: 100px;
float: left;
}
:matches(:matches(:matches([data-webkit]))) {
background-color: red;
}
:matches(:matches(:matches([data-webkit=foobar]))) {
background-color: blue;
}
</style>
</head>
<body>
<p>Verify style sharing does not ignore data attributes.</p>
<div>
<div>No data-webkit</div>
<div data-webkit>data-webkit defined</div>
<div data-webkit="">data-webkit defined empty</div>
<div data-webkit="foobar">data-webkit is foobar</div>
</div>
<div>
<span>No data-webkit</span>
<span data-webkit>data-webkit defined</span>
<span data-webkit="">data-webkit defined empty</span>
<span data-webkit="foobar">data-webkit is foobar</span>
</div>
<div>
<input value="No data-webkit">
<input data-webkit value="data-webkit defined">
<input data-webkit="" value="data-webkit defined empty">
<input data-webkit="foobar" value="data-webkit is foobar">
</div>
</body>
</html>

0 comments on commit 1234b65

Please sign in to comment.