Skip to content

Commit ceb69cd

Browse files
committed
Bug 1647845 Part 1 - Ignore any properties which aren't valid for a style rule when generating CSS2Properties and testing properties r=emilio
To know the valid rules for each property, we need to put this information into the Servo prop list and add an appropriate getter to Longhand/Shorthand. Differential Revision: https://phabricator.services.mozilla.com/D105825
1 parent 52d3d1e commit ceb69cd

File tree

7 files changed

+81
-9
lines changed

7 files changed

+81
-9
lines changed

dom/bindings/GenerateCSS2PropertiesWebIDL.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ def generate(output, idlFilename, dataFile):
2525
for p in propList:
2626
if "Internal" in p.flags:
2727
continue
28+
29+
# Skip properties which aren't valid in style rules.
30+
if "Style" not in p.rules:
31+
continue
32+
2833
# Unfortunately, even some of the getters here are fallible
2934
# (e.g. on nsComputedDOMStyle).
3035
extendedAttrs = [

layout/style/GenerateServoCSSPropList.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,20 @@ def generate_header(output, data):
100100
if not flags:
101101
flags = "CSSPropFlags(0)"
102102
params = [prop.name, prop.id, method, flags, pref]
103-
103+
excludes = []
104104
if is_internal:
105-
output.write("#ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL\n")
105+
excludes.append("CSS_PROP_LIST_EXCLUDE_INTERNAL")
106+
if "Style" not in prop.rules:
107+
excludes.append("CSS_PROP_LIST_EXCLUDE_NOT_IN_STYLE")
108+
109+
if excludes:
110+
output.write(
111+
"#if {}\n".format(
112+
" || ".join("!defined " + exclude for exclude in excludes)
113+
)
114+
)
106115
output.write("{}({})\n".format(MACRO_NAMES[prop.type()], ", ".join(params)))
107-
if is_internal:
116+
if excludes:
108117
output.write("#endif\n")
109118

110119
output.write(

layout/style/ServoCSSPropList.mako.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def _assign_slots(obj, args):
88

99

1010
class Longhand(object):
11-
__slots__ = ["name", "method", "id", "flags", "pref"]
11+
__slots__ = ["name", "method", "id", "rules", "flags", "pref"]
1212

1313
def __init__(self, *args):
1414
_assign_slots(self, args)
@@ -19,7 +19,7 @@ def type():
1919

2020

2121
class Shorthand(object):
22-
__slots__ = ["name", "method", "id", "flags", "pref", "subprops"]
22+
__slots__ = ["name", "method", "id", "rules", "flags", "pref", "subprops"]
2323

2424
def __init__(self, *args):
2525
_assign_slots(self, args)
@@ -30,7 +30,7 @@ def type():
3030

3131

3232
class Alias(object):
33-
__slots__ = ["name", "method", "alias_id", "prop_id", "flags", "pref"]
33+
__slots__ = ["name", "method", "alias_id", "prop_id", "rules", "flags", "pref"]
3434

3535
def __init__(self, *args):
3636
_assign_slots(self, args)
@@ -123,6 +123,9 @@ def exposed_on_getcs(prop):
123123
if prop.type() == "shorthand":
124124
return "SHORTHAND_IN_GETCS" in prop.flags
125125

126+
def rules(prop):
127+
return ", ".join('"{}"'.format(rule) for rule in prop.rule_types_allowed_names())
128+
126129
def flags(prop):
127130
result = []
128131
if prop.explicitly_enabled_in_chrome():
@@ -154,15 +157,15 @@ def sub_properties(prop):
154157

155158
data = [
156159
% for prop in data.longhands:
157-
Longhand("${prop.name}", "${method(prop)}", "${prop.ident}", [${flags(prop)}], ${pref(prop)}),
160+
Longhand("${prop.name}", "${method(prop)}", "${prop.ident}", [${rules(prop)}], [${flags(prop)}], ${pref(prop)}),
158161
% endfor
159162

160163
% for prop in data.shorthands:
161-
Shorthand("${prop.name}", "${prop.camel_case}", "${prop.ident}", [${flags(prop)}], ${pref(prop)},
164+
Shorthand("${prop.name}", "${prop.camel_case}", "${prop.ident}", [${rules(prop)}], [${flags(prop)}], ${pref(prop)},
162165
[${sub_properties(prop)}]),
163166
% endfor
164167

165168
% for prop in data.all_aliases():
166-
Alias("${prop.name}", "${prop.camel_case}", "${prop.ident}", "${prop.original.ident}", [], ${pref(prop)}),
169+
Alias("${prop.name}", "${prop.camel_case}", "${prop.ident}", "${prop.original.ident}", [${rules(prop)}], [], ${pref(prop)}),
167170
% endfor
168171
]

layout/style/nsDOMCSSDeclaration.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ class nsDOMCSSDeclaration : public nsICSSDeclaration {
107107
}
108108

109109
#define CSS_PROP_LIST_EXCLUDE_INTERNAL
110+
#define CSS_PROP_LIST_EXCLUDE_NOT_IN_STYLE
110111
#define CSS_PROP_LONGHAND(name_, id_, method_, ...) CSS_PROP(id_, method_)
111112
#define CSS_PROP_SHORTHAND(name_, id_, method_, ...) CSS_PROP(id_, method_)
112113
#define CSS_PROP_ALIAS(name_, aliasid_, id_, method_, ...) \
@@ -116,6 +117,7 @@ class nsDOMCSSDeclaration : public nsICSSDeclaration {
116117
#undef CSS_PROP_SHORTHAND
117118
#undef CSS_PROP_LONGHAND
118119
#undef CSS_PROP_LIST_EXCLUDE_INTERNAL
120+
#undef CSS_PROP_LIST_EXCLUDE_NOT_IN_STYLE
119121
#undef CSS_PROP
120122
#undef CSS_PROP_PUBLIC_OR_PRIVATE
121123

layout/style/test/ListCSSProperties.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
#include <stdlib.h>
1111
#include "mozilla/ArrayUtils.h"
1212

13+
// Do not consider properties not valid in style rules
14+
#define CSS_PROP_LIST_EXCLUDE_NOT_IN_STYLE
15+
1316
// Need an extra level of macro nesting to force expansion of method_
1417
// params before they get pasted.
1518
#define STRINGIFY_METHOD(method_) #method_

servo/components/style/properties/data.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,11 @@ def __init__(
261261
self.extra_prefixes = parse_property_aliases(extra_prefixes)
262262
self.flags = flags.split() if flags else []
263263

264+
def rule_types_allowed_names(self):
265+
for name in RULE_VALUES:
266+
if self.rule_types_allowed & RULE_VALUES[name] != 0:
267+
yield name
268+
264269
def experimental(self, engine):
265270
if engine == "gecko":
266271
return bool(self.gecko_pref)
@@ -598,6 +603,11 @@ def __init__(self, name, original, gecko_pref):
598603
def type():
599604
return "alias"
600605

606+
def rule_types_allowed_names(self):
607+
for name in RULE_VALUES:
608+
if self.rule_types_allowed & RULE_VALUES[name] != 0:
609+
yield name
610+
601611
def experimental(self, engine):
602612
if engine == "gecko":
603613
return bool(self.gecko_pref)
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<!DOCTYPE HTML>
2+
<html>
3+
<head>
4+
<link rel="author" title="Mozilla" href="https://mozilla.org">
5+
<link rel="help" href="https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface">
6+
<title>Page descriptors shouldn't be exposed to CSS Style declarations</title>
7+
<script src="/resources/testharness.js"></script>
8+
<script src="/resources/testharnessreport.js"></script>
9+
</head>
10+
<body>
11+
<div id="target"></div>
12+
<script>
13+
'use strict';
14+
15+
let element = document.getElementById("target");
16+
let computedStyle = window.getComputedStyle(element);
17+
let style = element.style;
18+
19+
test(t => {
20+
assert_equals(computedStyle.size, undefined,
21+
"computed style should not have size property");
22+
assert_equals(computedStyle.getPropertyValue("size"), "",
23+
"computed style getPropertyValue(\"size\") should be empty");
24+
25+
assert_equals(style.size, undefined,
26+
"style should not have size property");
27+
assert_equals(style.getPropertyValue("size"), "",
28+
"style getPropertyValue(\"size\") should be empty");
29+
for(const val of ["initial", "auto", "100px"]){
30+
style.setProperty("size", val);
31+
assert_false(CSS.supports("size", val));
32+
assert_equals(style.size, undefined,
33+
"style should not have size property after assigning size=" + val);
34+
assert_equals(style.getPropertyValue("size"), "",
35+
"style getPropertyValue(\"size\") should be empty after assigning size=" + val);
36+
}
37+
});
38+
</script>
39+
</body>
40+
</html>

0 commit comments

Comments
 (0)