Skip to content

Commit

Permalink
Put presentational hints into their own origin
Browse files Browse the repository at this point in the history 
Per https://drafts.csswg.org/css-cascade-5/#preshint, presentational
hints should be treated as in an independent origin for the purpose of
cascading, and part of the author origin for the `revert` keyword.

This patch implements that. It also fixes the behavior when
'revert-layer' should revert to presentational hints.

Fixed: 1306511
Change-Id: Iff98d5dfc63f8b2a108a74c5d20710f9ec9cdd90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3534543
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983889}
  • Loading branch information
@xiaochengh
xiaochengh authored and Chromium LUCI CQ committed Mar 22, 2022
1 parent 833b78d commit 59f0876
Show file tree
Hide file tree
Showing 13 changed files with 136 additions and 19 deletions.
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/css/element_rule_collector.h
View file
Expand Up @@ -140,6 +140,9 @@ class CORE_EXPORT ElementRuleCollector {
void FinishAddingUserRules() {
result_.FinishAddingUserRules();
}
void FinishAddingPresentationalHints() {
result_.FinishAddingPresentationalHints();
}
void FinishAddingAuthorRulesForTreeScope(const TreeScope& tree_scope) {
result_.FinishAddingAuthorRulesForTreeScope(tree_scope);
}
Expand Down
27 changes: 27 additions & 0 deletions third_party/blink/renderer/core/css/resolver/cascade_expansion_test.cc
View file
Expand Up @@ -85,6 +85,7 @@ TEST_F(CascadeExpansionTest, UARules) {
result.AddMatchedProperties(ParseDeclarationBlock("cursor:help;top:1px"));
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(1u, result.GetMatchedProperties().size());
Expand All @@ -107,6 +108,7 @@ TEST_F(CascadeExpansionTest, UserRules) {
result.AddMatchedProperties(ParseDeclarationBlock("cursor:help"));
result.AddMatchedProperties(ParseDeclarationBlock("float:left"));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(2u, result.GetMatchedProperties().size());
Expand Down Expand Up @@ -134,6 +136,7 @@ TEST_F(CascadeExpansionTest, AuthorRules) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("cursor:help;top:1px"));
result.AddMatchedProperties(ParseDeclarationBlock("float:left"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand Down Expand Up @@ -169,6 +172,7 @@ TEST_F(CascadeExpansionTest, AllOriginRules) {
result.FinishAddingUARules();
result.AddMatchedProperties(ParseDeclarationBlock("cursor:help;top:1px"));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("left:1px"));
result.AddMatchedProperties(ParseDeclarationBlock("float:left"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand Down Expand Up @@ -231,6 +235,7 @@ TEST_F(CascadeExpansionTest, Name) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("--x:1px;--y:2px"));
result.AddMatchedProperties(ParseDeclarationBlock("float:left"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand Down Expand Up @@ -265,6 +270,7 @@ TEST_F(CascadeExpansionTest, Value) {
result.AddMatchedProperties(ParseDeclarationBlock("background-color:red"));
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(1u, result.GetMatchedProperties().size());
Expand All @@ -285,6 +291,7 @@ TEST_F(CascadeExpansionTest, LinkOmitted) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("color:red"),
AddMatchedPropertiesOptions::Builder()
.SetLinkMatchType(CSSSelector::kMatchVisited)
Expand All @@ -304,6 +311,7 @@ TEST_F(CascadeExpansionTest, InternalVisited) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("color:red"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -323,6 +331,7 @@ TEST_F(CascadeExpansionTest, InternalVisitedOmitted) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("color:red"),
AddMatchedPropertiesOptions::Builder()
.SetLinkMatchType(CSSSelector::kMatchLink)
Expand All @@ -342,6 +351,7 @@ TEST_F(CascadeExpansionTest, InternalVisitedWithTrailer) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("color:red;left:1px"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -364,6 +374,7 @@ TEST_F(CascadeExpansionTest, All) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("all:unset"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -384,6 +395,7 @@ TEST_F(CascadeExpansionTest, InlineAll) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("left:1px;all:unset;right:1px"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand Down Expand Up @@ -413,6 +425,7 @@ TEST_F(CascadeExpansionTest, FilterNormalNonInherited) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("font-size:1px;left:1px"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -433,6 +446,7 @@ TEST_F(CascadeExpansionTest, FilterInternalVisited) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("color:red"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -451,6 +465,7 @@ TEST_F(CascadeExpansionTest, FilterFirstLetter) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("object-fit:unset;font-size:1px"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -469,6 +484,7 @@ TEST_F(CascadeExpansionTest, FilterFirstLine) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("display:none;font-size:1px"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -487,6 +503,7 @@ TEST_F(CascadeExpansionTest, FilterCue) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("object-fit:unset;font-size:1px"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -505,6 +522,7 @@ TEST_F(CascadeExpansionTest, FilterMarker) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("object-fit:unset;font-size:1px"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -523,6 +541,7 @@ TEST_F(CascadeExpansionTest, FilterHighlight) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("display:block;background-color:lime;"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -544,6 +563,7 @@ TEST_F(CascadeExpansionTest, FilterAllNonInherited) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("all:unset"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -566,6 +586,7 @@ TEST_F(CascadeExpansionTest, Importance) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("cursor:help;display:block !important"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand All @@ -590,6 +611,7 @@ TEST_F(CascadeExpansionTest, AllImportance) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("all:unset !important"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -611,6 +633,7 @@ TEST_F(CascadeExpansionTest, AllNonImportance) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("all:unset"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

Expand All @@ -632,6 +655,7 @@ TEST_F(CascadeExpansionTest, AllVisitedOnly) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("all:unset"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -656,6 +680,7 @@ TEST_F(CascadeExpansionTest, AllVisitedOrLink) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("all:unset"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -680,6 +705,7 @@ TEST_F(CascadeExpansionTest, AllLinkOnly) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(
ParseDeclarationBlock("all:unset"),
AddMatchedPropertiesOptions::Builder()
Expand All @@ -699,6 +725,7 @@ TEST_F(CascadeExpansionTest, Position) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("left:1px;top:1px"));
result.AddMatchedProperties(ParseDeclarationBlock("bottom:1px;right:1px"));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand Down
8 changes: 5 additions & 3 deletions third_party/blink/renderer/core/css/resolver/cascade_origin.h
View file
Expand Up @@ -16,13 +16,15 @@ enum class CascadeOrigin : uint8_t {
kNone = 0,
kUserAgent = 0b0001,
kUser = 0b0010,
kAuthor = 0b0011,
kAnimation = 0b0100,
// https://drafts.csswg.org/css-cascade-5/#preshint
kAuthorPresentationalHint = 0b0011,
kAuthor = 0b0100,
kAnimation = 0b0101,
// The lower four bits of kAuthor, kUser and kUserAgent can be inverted to
// efficiently produce a "cascade correct" value when compared with the values
// specified in this enum:
//
// kAuthor important: ~0b0011 == 0b1100 (> kAnimation)
// kAuthor important: ~0b0100 == 0b1011 (> kAnimation)
// kUser important: ~0b0010 == 0b1101 (> kAuthor important)
// kUserAgent important: ~0b0001 == 0b1110 (> kUser important)
//
Expand Down
6 changes: 3 additions & 3 deletions third_party/blink/renderer/core/css/resolver/cascade_priority_test.cc
View file
Expand Up @@ -26,9 +26,9 @@ TEST(CascadePriorityTest, EncodeOriginImportance) {
using Origin = CascadeOrigin;
EXPECT_EQ(0b00001ull, EncodeOriginImportance(Origin::kUserAgent, false));
EXPECT_EQ(0b00010ull, EncodeOriginImportance(Origin::kUser, false));
EXPECT_EQ(0b00011ull, EncodeOriginImportance(Origin::kAuthor, false));
EXPECT_EQ(0b00100ull, EncodeOriginImportance(Origin::kAnimation, false));
EXPECT_EQ(0b01100ull, EncodeOriginImportance(Origin::kAuthor, true));
EXPECT_EQ(0b00100ull, EncodeOriginImportance(Origin::kAuthor, false));
EXPECT_EQ(0b00101ull, EncodeOriginImportance(Origin::kAnimation, false));
EXPECT_EQ(0b01011ull, EncodeOriginImportance(Origin::kAuthor, true));
EXPECT_EQ(0b01101ull, EncodeOriginImportance(Origin::kUser, true));
EXPECT_EQ(0b01110ull, EncodeOriginImportance(Origin::kUserAgent, true));
EXPECT_EQ(0b10000ull, EncodeOriginImportance(Origin::kTransition, false));
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/css/resolver/match_result.cc
View file
Expand Up @@ -72,6 +72,11 @@ void MatchResult::FinishAddingUARules() {

void MatchResult::FinishAddingUserRules() {
DCHECK_EQ(current_origin_, CascadeOrigin::kUser);
current_origin_ = CascadeOrigin::kAuthorPresentationalHint;
}

void MatchResult::FinishAddingPresentationalHints() {
DCHECK_EQ(current_origin_, CascadeOrigin::kAuthorPresentationalHint);
current_origin_ = CascadeOrigin::kAuthor;
}

Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/css/resolver/match_result.h
View file
Expand Up @@ -196,6 +196,7 @@ class CORE_EXPORT MatchResult {

void FinishAddingUARules();
void FinishAddingUserRules();
void FinishAddingPresentationalHints();
void FinishAddingAuthorRulesForTreeScope(const TreeScope&);

void SetIsCacheable(bool cacheable) { is_cacheable_ = cacheable; }
Expand Down
14 changes: 14 additions & 0 deletions third_party/blink/renderer/core/css/resolver/match_result_test.cc
View file
Expand Up @@ -57,6 +57,7 @@ TEST_F(MatchResultTest, CascadeOriginUserAgent) {
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(LengthOf(result), 2u);
Expand All @@ -70,6 +71,7 @@ TEST_F(MatchResultTest, CascadeOriginUser) {
result.AddMatchedProperties(PropertySet(0));
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(LengthOf(result), 2u);
Expand All @@ -81,6 +83,7 @@ TEST_F(MatchResultTest, CascadeOriginAuthor) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(0));
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
Expand All @@ -97,6 +100,7 @@ TEST_F(MatchResultTest, CascadeOriginAll) {
result.AddMatchedProperties(PropertySet(1));
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(3));
result.AddMatchedProperties(PropertySet(4));
result.AddMatchedProperties(PropertySet(5));
Expand All @@ -117,6 +121,7 @@ TEST_F(MatchResultTest, CascadeOriginAllExceptUserAgent) {
result.AddMatchedProperties(PropertySet(1));
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(3));
result.AddMatchedProperties(PropertySet(4));
result.AddMatchedProperties(PropertySet(5));
Expand All @@ -135,6 +140,7 @@ TEST_F(MatchResultTest, CascadeOriginAllExceptUser) {
result.AddMatchedProperties(PropertySet(0));
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(3));
result.AddMatchedProperties(PropertySet(4));
result.AddMatchedProperties(PropertySet(5));
Expand All @@ -154,6 +160,7 @@ TEST_F(MatchResultTest, CascadeOriginAllExceptAuthor) {
result.AddMatchedProperties(PropertySet(1));
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

ASSERT_EQ(LengthOf(result), 3u);
Expand All @@ -168,6 +175,7 @@ TEST_F(MatchResultTest, CascadeOriginTreeScopes) {
result.FinishAddingUARules();
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
result.AddMatchedProperties(PropertySet(3));
Expand Down Expand Up @@ -196,6 +204,7 @@ TEST_F(MatchResultTest, ExpansionsRange) {
result.FinishAddingUARules();
result.AddMatchedProperties(ParseDeclarationBlock("display:block"));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(ParseDeclarationBlock("left:unset"));
result.AddMatchedProperties(ParseDeclarationBlock("top:unset"));
result.AddMatchedProperties(
Expand Down Expand Up @@ -223,6 +232,7 @@ TEST_F(MatchResultTest, EmptyExpansionsRange) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.FinishAddingAuthorRulesForTreeScope(GetDocument());

CascadeFilter filter;
Expand All @@ -236,6 +246,7 @@ TEST_F(MatchResultTest, Reset) {
result.FinishAddingUARules();
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
result.AddMatchedProperties(PropertySet(3));
Expand Down Expand Up @@ -270,6 +281,7 @@ TEST_F(MatchResultTest, Reset) {
result.FinishAddingUARules();
result.AddMatchedProperties(PropertySet(1));
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(2));
result.FinishAddingAuthorRulesForTreeScope(GetDocument());
result.AddMatchedProperties(PropertySet(3));
Expand Down Expand Up @@ -304,6 +316,7 @@ TEST_F(MatchResultTest, ResetTreeScope) {
MatchResult result;
result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(0));
result.FinishAddingAuthorRulesForTreeScope(scope1);

Expand All @@ -314,6 +327,7 @@ TEST_F(MatchResultTest, ResetTreeScope) {

result.FinishAddingUARules();
result.FinishAddingUserRules();
result.FinishAddingPresentationalHints();
result.AddMatchedProperties(PropertySet(0));
result.FinishAddingAuthorRulesForTreeScope(scope2);

Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/css/resolver/matched_properties_cache_test.cc
View file
Expand Up @@ -32,6 +32,7 @@ class MatchedPropertiesCacheTestKey {
const TreeScope& tree_scope) {
result_.FinishAddingUARules();
result_.FinishAddingUserRules();
result_.FinishAddingPresentationalHints();
auto* set = css_test_helpers::ParseDeclarationBlock(block_text);
result_.AddMatchedProperties(set);
result_.FinishAddingAuthorRulesForTreeScope(tree_scope);
Expand Down

0 comments on commit 59f0876

Please sign in to comment.