Commit 843f972e authored by mstensho@opera.com's avatar mstensho@opera.com

Applying the "all" property needs to check all properties against the whitelist.

When applying any other property than "all", we already perform this whitelist
check, but applying "all" is a separate code path, and the checks were missing
there.

BUG=524682
R=rune@opera.com

Review URL: https://codereview.chromium.org/1304123006

git-svn-id: svn://svn.chromium.org/blink/trunk@201315 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent c6071473
<!DOCTYPE html>
<style>
#elm { overflow:scroll; max-width:4em; padding-left:3em; opacity:0.6; background:yellow; }
</style>
<p>Test that an all:inherit declaration in a ::first-letter rule inherits what should be inherited,
while invalid properties for ::first-letter aren't.</p>
<p>The word "PASS" should be seen below, in a yellow box. There should be scrollbars on the yellow
box. Translucency should be uniform. There should be no red.</p>
<div id="elm">PASS</div>
<!DOCTYPE html>
<style>
/* Some invalid properties for ::first-letter */
#elm { overflow:scroll; max-width:4em; opacity:0.6; background:yellow; }
/* Some valid properties for ::first-letter */
#elm { margin-right:10px; padding-left:3em; }
#elm::first-letter { color:red; all:inherit; margin-left:-3em; float:left; }
</style>
<p>Test that an all:inherit declaration in a ::first-letter rule inherits what should be inherited,
while invalid properties for ::first-letter aren't.</p>
<p>The word "PASS" should be seen below, in a yellow box. There should be scrollbars on the yellow
box. Translucency should be uniform. There should be no red.</p>
<div id="elm">P<span style="margin-left:-10px;">ASS</span></div>
overflow-y isn't a valid property for ::first-letter. We'd crash when extracting innerText, because the implementation expects that the first letter text is a direct child of the ::first-letter pseudo object. The paged overflow / multicol implementation would violate this assumption by inserting a flow thread object between the ::first-letter pseudo object and the actual text.
PASS if no crash or assertion failure.
x
<!DOCTYPE html>
<style>
#elm { overflow-y:-webkit-paged-y; }
/* Need to float the first letter, to turn it into a block. Otherwise, overflow-y wouldn't be
an applicable property in the first place. */
#elm::first-letter { all:inherit; float:left; }
</style>
<p>overflow-y isn't a valid property for ::first-letter. We'd crash when extracting innerText,
because the implementation expects that the first letter text is a direct child of the
::first-letter pseudo object. The paged overflow / multicol implementation would violate this
assumption by inserting a flow thread object between the ::first-letter pseudo object and the
actual text.</p>
<p>PASS if no crash or assertion failure.</p>
<div id="elm">x</div>
<script>
if (window.testRunner)
testRunner.dumpAsText();
document.getElementById("elm").innerText;
</script>
...@@ -1195,7 +1195,7 @@ static inline bool isValidFirstLetterStyleProperty(CSSPropertyID id) ...@@ -1195,7 +1195,7 @@ static inline bool isValidFirstLetterStyleProperty(CSSPropertyID id)
} }
} }
static bool shouldIgnoreTextTrackAuthorStyle(Document& document) static bool shouldIgnoreTextTrackAuthorStyle(const Document& document)
{ {
Settings* settings = document.settings(); Settings* settings = document.settings();
if (!settings) if (!settings)
...@@ -1212,10 +1212,25 @@ static bool shouldIgnoreTextTrackAuthorStyle(Document& document) ...@@ -1212,10 +1212,25 @@ static bool shouldIgnoreTextTrackAuthorStyle(Document& document)
return false; return false;
} }
static inline bool isPropertyInWhitelist(PropertyWhitelistType propertyWhitelistType, CSSPropertyID property, const Document& document)
{
if (propertyWhitelistType == PropertyWhitelistNone)
return true; // Early bail for the by far most common case.
if (propertyWhitelistType == PropertyWhitelistFirstLetter)
return isValidFirstLetterStyleProperty(property);
if (propertyWhitelistType == PropertyWhitelistCue)
return isValidCueStyleProperty(property) && !shouldIgnoreTextTrackAuthorStyle(document);
ASSERT_NOT_REACHED();
return true;
}
// This method expands the 'all' shorthand property to longhand properties // This method expands the 'all' shorthand property to longhand properties
// and applies the expanded longhand properties. // and applies the expanded longhand properties.
template <CSSPropertyPriority priority> template <CSSPropertyPriority priority>
void StyleResolver::applyAllProperty(StyleResolverState& state, CSSValue* allValue, bool inheritedOnly) void StyleResolver::applyAllProperty(StyleResolverState& state, CSSValue* allValue, bool inheritedOnly, PropertyWhitelistType propertyWhitelistType)
{ {
unsigned startCSSProperty = CSSPropertyPriorityData<priority>::first(); unsigned startCSSProperty = CSSPropertyPriorityData<priority>::first();
unsigned endCSSProperty = CSSPropertyPriorityData<priority>::last(); unsigned endCSSProperty = CSSPropertyPriorityData<priority>::last();
...@@ -1236,6 +1251,9 @@ void StyleResolver::applyAllProperty(StyleResolverState& state, CSSValue* allVal ...@@ -1236,6 +1251,9 @@ void StyleResolver::applyAllProperty(StyleResolverState& state, CSSValue* allVal
if (!CSSProperty::isAffectedByAllProperty(propertyId)) if (!CSSProperty::isAffectedByAllProperty(propertyId))
continue; continue;
if (!isPropertyInWhitelist(propertyWhitelistType, propertyId, document()))
continue;
// When hitting matched properties' cache, only inherited properties will be // When hitting matched properties' cache, only inherited properties will be
// applied. // applied.
if (inheritedOnly && !CSSPropertyMetadata::isInheritedProperty(propertyId)) if (inheritedOnly && !CSSPropertyMetadata::isInheritedProperty(propertyId))
...@@ -1256,14 +1274,11 @@ void StyleResolver::applyProperties(StyleResolverState& state, const StyleProper ...@@ -1256,14 +1274,11 @@ void StyleResolver::applyProperties(StyleResolverState& state, const StyleProper
CSSPropertyID property = current.id(); CSSPropertyID property = current.id();
if (property == CSSPropertyAll) { if (property == CSSPropertyAll) {
applyAllProperty<priority>(state, current.value(), inheritedOnly); applyAllProperty<priority>(state, current.value(), inheritedOnly, propertyWhitelistType);
continue; continue;
} }
if (propertyWhitelistType == PropertyWhitelistCue && (!isValidCueStyleProperty(property) || shouldIgnoreTextTrackAuthorStyle(document()))) if (!isPropertyInWhitelist(propertyWhitelistType, property, document()))
continue;
if (propertyWhitelistType == PropertyWhitelistFirstLetter && !isValidFirstLetterStyleProperty(property))
continue; continue;
if (inheritedOnly && !current.isInherited()) { if (inheritedOnly && !current.isInherited()) {
......
...@@ -217,7 +217,7 @@ private: ...@@ -217,7 +217,7 @@ private:
template <CSSPropertyPriority priority> template <CSSPropertyPriority priority>
void applyAnimatedProperties(StyleResolverState&, const WillBeHeapHashMap<PropertyHandle, RefPtrWillBeMember<Interpolation>>&); void applyAnimatedProperties(StyleResolverState&, const WillBeHeapHashMap<PropertyHandle, RefPtrWillBeMember<Interpolation>>&);
template <CSSPropertyPriority priority> template <CSSPropertyPriority priority>
void applyAllProperty(StyleResolverState&, CSSValue*, bool inheritedOnly); void applyAllProperty(StyleResolverState&, CSSValue*, bool inheritedOnly, PropertyWhitelistType);
bool pseudoStyleForElementInternal(Element&, const PseudoStyleRequest&, const ComputedStyle* parentStyle, StyleResolverState&); bool pseudoStyleForElementInternal(Element&, const PseudoStyleRequest&, const ComputedStyle* parentStyle, StyleResolverState&);
bool hasAuthorBackground(const StyleResolverState&); bool hasAuthorBackground(const StyleResolverState&);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment