Commit 73513536 authored by Darren Shen's avatar Darren Shen Committed by Commit Bot

[css-typed-om] Implement shorthands for StylePropertyMap.set/delete.

This patch implements shorthands for stylemap.set/delete. When we
receive an UnsupportedShorthandProperty, we should get the CSSValues
out of them and set them on the style map.

We leave parsing of shorthand values for a future patch.

Bug: 816722
Change-Id: Iebd0b33f89b1d8872bea5d3f80d1dde71d72bc45
Reviewed-on: https://chromium-review.googlesource.com/938684
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: default avatarnainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539992}
parent 5ccf96ac
...@@ -13,6 +13,7 @@ const gInvalidTestCases = [ ...@@ -13,6 +13,7 @@ const gInvalidTestCases = [
{ property: 'lemon', values: ['ade'], desc: 'an unsupported property name' }, { property: 'lemon', values: ['ade'], desc: 'an unsupported property name' },
{ property: null, values: ['foo'], desc: 'an null property name' }, { property: null, values: ['foo'], desc: 'an null property name' },
{ property: 'width', values: ['10px'], desc: 'a property that is not list valued' }, { property: 'width', values: ['10px'], desc: 'a property that is not list valued' },
{ property: 'margin', values: ['10px'], desc: 'a shorthand property' },
{ property: 'transition-duration', values: [CSS.px(10)], desc: 'an invalid CSSStyleValue' }, { property: 'transition-duration', values: [CSS.px(10)], desc: 'an invalid CSSStyleValue' },
{ property: 'transition-duration', values: ['10px'], desc: 'an invalid String value' }, { property: 'transition-duration', values: ['10px'], desc: 'an invalid String value' },
{ property: 'transition-duration', values: [CSS.s(1), '10px', CSS.px(10)], desc: 'a mix of valid and invalid values' }, { property: 'transition-duration', values: [CSS.s(1), '10px', CSS.px(10)], desc: 'a mix of valid and invalid values' },
......
<!doctype html>
<meta charset="utf-8">
<title>Declared StylePropertyMap.delete() with shorthands</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#delete-a-stylepropertymap">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<body>
<div id="log">
<script>
'use strict';
test(t => {
let [elem, styleMap] = createRuleWithDeclaredStyleMap(t, '');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
styleMap.delete('margin');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
}, 'Deleting a shorthand property not in the css rule is a no-op');
test(t => {
let [elem, styleMap] = createRuleWithDeclaredStyleMap(t, 'margin: 10px');
assert_not_equals(elem.style.getPropertyValue('margin'), '');
styleMap.delete('margin');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
}, 'Deleting a shorthand property in the css rule removes both it and ' +
'its longhands');
test(t => {
let [elem, styleMap] = createRuleWithDeclaredStyleMap(t, 'margin: 10px');
assert_not_equals(elem.style.getPropertyValue('margin-top'), '');
styleMap.delete('margin-top');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '10px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '10px');
assert_equals(elem.style.getPropertyValue('margin-right'), '10px');
}, 'Deleting a longhand property in the css rule removes both it and ' +
'its shorthand');
</script>
This is a testharness.js-based test.
PASS Setting a shorthand with an invalid CSSStyleValue on css rule throws TypeError
PASS Setting a shorthand with an invalid String on css rule throws TypeError
PASS Setting a shorthand with a CSSStyleValue updates css rule
FAIL Setting a shorthand with a string updates css rule Failed to execute 'set' on 'StylePropertyMap': Parsing shorthands is not supported yet
Harness: the test ran to completion.
<!doctype html>
<meta charset="utf-8">
<title>Declared StylePropertyMap.set() with shorthands</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#set-a-value-on-a-stylepropertymap">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<body>
<script>
'use strict';
const gInvalidTestCases = [
{ property: 'margin', values: [CSS.deg(0)], desc: 'an invalid CSSStyleValue' },
{ property: 'margin', values: ['10s'], desc: 'an invalid String' },
];
for (const {property, values, desc} of gInvalidTestCases) {
test(t => {
let styleMap = createInlineStyleMap(t, '');
assert_throws(new TypeError(), () => styleMap.set(property, ...values));
}, 'Setting a shorthand with ' + desc + ' on css rule throws TypeError');
}
test(t => {
let [elem, styleMap] = createRuleWithDeclaredStyleMap(t, 'margin: 1px 2px 3px 4px');
const result = styleMap.get('margin');
elem.style.margin = '';
styleMap.set('margin', result);
assert_equals(elem.style.getPropertyValue('margin'), '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin-top'), '1px');
assert_equals(elem.style.getPropertyValue('margin-right'), '2px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '3px');
assert_equals(elem.style.getPropertyValue('margin-left'), '4px');
}, 'Setting a shorthand with a CSSStyleValue updates css rule');
test(t => {
let [elem, styleMap] = createRuleWithDeclaredStyleMap(t);
styleMap.set('margin', '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin'), '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin-top'), '1px');
assert_equals(elem.style.getPropertyValue('margin-right'), '2px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '3px');
assert_equals(elem.style.getPropertyValue('margin-left'), '4px');
}, 'Setting a shorthand with a string updates css rule');
</script>
...@@ -13,6 +13,7 @@ const gInvalidTestCases = [ ...@@ -13,6 +13,7 @@ const gInvalidTestCases = [
{ property: 'lemon', values: ['ade'], desc: 'an unsupported property name' }, { property: 'lemon', values: ['ade'], desc: 'an unsupported property name' },
{ property: null, values: ['foo'], desc: 'an null property name' }, { property: null, values: ['foo'], desc: 'an null property name' },
{ property: 'width', values: ['10px'], desc: 'a property that is not list valued' }, { property: 'width', values: ['10px'], desc: 'a property that is not list valued' },
{ property: 'margin', values: ['10px'], desc: 'a shorthand property' },
{ property: 'transition-duration', values: [CSS.px(10)], desc: 'an invalid CSSStyleValue' }, { property: 'transition-duration', values: [CSS.px(10)], desc: 'an invalid CSSStyleValue' },
{ property: 'transition-duration', values: ['10px'], desc: 'an invalid String value' }, { property: 'transition-duration', values: ['10px'], desc: 'an invalid String value' },
{ property: 'transition-duration', values: [CSS.s(1), '10px', CSS.px(10)], desc: 'a mix of valid and invalid values' }, { property: 'transition-duration', values: [CSS.s(1), '10px', CSS.px(10)], desc: 'a mix of valid and invalid values' },
......
<!doctype html>
<meta charset="utf-8">
<title>Inline StylePropertyMap.delete() with shorthands</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#delete-a-stylepropertymap">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<body>
<div id="log">
<script>
'use strict';
test(t => {
let [elem, styleMap] = createElementWithInlineStyleMap(t, '');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
styleMap.delete('margin');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
}, 'Deleting a shorthand property not in the inline style is a no-op');
test(t => {
let [elem, styleMap] = createElementWithInlineStyleMap(t, 'margin: 10px');
assert_not_equals(elem.style.getPropertyValue('margin'), '');
styleMap.delete('margin');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '');
assert_equals(elem.style.getPropertyValue('margin-right'), '');
}, 'Deleting a shorthand property in the inline style removes both it and ' +
'its longhands');
test(t => {
let [elem, styleMap] = createElementWithInlineStyleMap(t, 'margin: 10px');
assert_not_equals(elem.style.getPropertyValue('margin-top'), '');
styleMap.delete('margin-top');
assert_equals(elem.style.getPropertyValue('margin'), '');
assert_equals(elem.style.getPropertyValue('margin-top'), '');
assert_equals(elem.style.getPropertyValue('margin-left'), '10px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '10px');
assert_equals(elem.style.getPropertyValue('margin-right'), '10px');
}, 'Deleting a longhand property in the inline style removes both it and ' +
'its shorthand');
</script>
This is a testharness.js-based test.
PASS Setting a shorthand with an invalid CSSStyleValue on inline style throws TypeError
PASS Setting a shorthand with an invalid String on inline style throws TypeError
PASS Setting a shorthand with a CSSStyleValue updates inline style
FAIL Setting a shorthand with a string updates inline style Failed to execute 'set' on 'StylePropertyMap': Parsing shorthands is not supported yet
Harness: the test ran to completion.
<!doctype html>
<meta charset="utf-8">
<title>Inline StylePropertyMap.set() with shorthands</title>
<link rel="help" href="https://drafts.css-houdini.org/css-typed-om-1/#set-a-value-on-a-stylepropertymap">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../resources/testhelper.js"></script>
<body>
<script>
'use strict';
const gInvalidTestCases = [
{ property: 'margin', values: [CSS.deg(0)], desc: 'an invalid CSSStyleValue' },
{ property: 'margin', values: ['10s'], desc: 'an invalid String' },
];
for (const {property, values, desc} of gInvalidTestCases) {
test(t => {
let styleMap = createInlineStyleMap(t, '');
assert_throws(new TypeError(), () => styleMap.set(property, ...values));
}, 'Setting a shorthand with ' + desc + ' on inline style throws TypeError');
}
test(t => {
let [elem, styleMap] = createElementWithInlineStyleMap(t, 'margin: 1px 2px 3px 4px');
const result = styleMap.get('margin');
elem.style.margin = '';
styleMap.set('margin', result);
assert_equals(elem.style.getPropertyValue('margin'), '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin-top'), '1px');
assert_equals(elem.style.getPropertyValue('margin-right'), '2px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '3px');
assert_equals(elem.style.getPropertyValue('margin-left'), '4px');
}, 'Setting a shorthand with a CSSStyleValue updates inline style');
test(t => {
let [elem, styleMap] = createElementWithInlineStyleMap(t);
styleMap.set('margin', '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin'), '1px 2px 3px 4px');
assert_equals(elem.style.getPropertyValue('margin-top'), '1px');
assert_equals(elem.style.getPropertyValue('margin-right'), '2px');
assert_equals(elem.style.getPropertyValue('margin-bottom'), '3px');
assert_equals(elem.style.getPropertyValue('margin-left'), '4px');
}, 'Setting a shorthand with a string updates inline style');
</script>
...@@ -27,4 +27,8 @@ for (const suffix of ['top', 'left', 'right', 'bottom']) { ...@@ -27,4 +27,8 @@ for (const suffix of ['top', 'left', 'right', 'bottom']) {
]); ]);
} }
runUnsupportedPropertyTests('margin',
['1px', '1px 2px 3px 4px']
);
</script> </script>
...@@ -5,9 +5,11 @@ ...@@ -5,9 +5,11 @@
#include "core/css/cssom/StylePropertyMap.h" #include "core/css/cssom/StylePropertyMap.h"
#include "bindings/core/v8/ExceptionState.h" #include "bindings/core/v8/ExceptionState.h"
#include "core/StylePropertyShorthand.h"
#include "core/css/CSSValueList.h" #include "core/css/CSSValueList.h"
#include "core/css/cssom/CSSOMTypes.h" #include "core/css/cssom/CSSOMTypes.h"
#include "core/css/cssom/CSSStyleValue.h" #include "core/css/cssom/CSSStyleValue.h"
#include "core/css/cssom/CSSUnsupportedShorthandValue.h"
#include "core/css/cssom/StyleValueFactory.h" #include "core/css/cssom/StyleValueFactory.h"
#include "core/css/parser/CSSParserContext.h" #include "core/css/parser/CSSParserContext.h"
#include "core/css/properties/CSSProperty.h" #include "core/css/properties/CSSProperty.h"
...@@ -118,13 +120,30 @@ void StylePropertyMap::set(const ExecutionContext* execution_context, ...@@ -118,13 +120,30 @@ void StylePropertyMap::set(const ExecutionContext* execution_context,
const HeapVector<CSSStyleValueOrString>& values, const HeapVector<CSSStyleValueOrString>& values,
ExceptionState& exception_state) { ExceptionState& exception_state) {
const CSSPropertyID property_id = cssPropertyID(property_name); const CSSPropertyID property_id = cssPropertyID(property_name);
if (property_id == CSSPropertyInvalid) { if (property_id == CSSPropertyInvalid) {
exception_state.ThrowTypeError("Invalid propertyName: " + property_name); exception_state.ThrowTypeError("Invalid propertyName: " + property_name);
return; return;
} }
DCHECK(isValidCSSPropertyID(property_id));
const CSSProperty& property = CSSProperty::Get(property_id); const CSSProperty& property = CSSProperty::Get(property_id);
if (property.IsShorthand()) {
if (values.size() != 1) {
exception_state.ThrowTypeError("Invalid type for property");
return;
}
if (values[0].IsString()) {
exception_state.ThrowTypeError("Parsing shorthands is not supported yet");
return;
}
if (values[0].IsCSSStyleValue() &&
!SetShorthandProperty(property, *values[0].GetAsCSSStyleValue())) {
exception_state.ThrowTypeError("Invalid type for property");
return;
}
return;
}
const CSSValue* result = nullptr; const CSSValue* result = nullptr;
if (property.IsRepeated()) if (property.IsRepeated())
...@@ -211,4 +230,25 @@ void StylePropertyMap::clear() { ...@@ -211,4 +230,25 @@ void StylePropertyMap::clear() {
RemoveAllProperties(); RemoveAllProperties();
} }
bool StylePropertyMap::SetShorthandProperty(const CSSProperty& property,
const CSSStyleValue& value) {
if (value.GetType() != CSSStyleValue::kShorthandType)
return false;
const auto& shorthand_value = ToCSSUnsupportedShorthandValue(value);
if (shorthand_value.GetProperty() != property.PropertyID())
return false;
const StylePropertyShorthand& shorthand =
shorthandForProperty(property.PropertyID());
for (size_t i = 0; i < shorthand.length(); i++) {
if (shorthand_value.LonghandValues()[i]) {
SetProperty(shorthand.properties()[i]->PropertyID(),
*shorthand_value.LonghandValues()[i]);
}
}
return true;
}
} // namespace blink } // namespace blink
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
namespace blink { namespace blink {
class CSSProperty;
class ExceptionState; class ExceptionState;
class ExecutionContext; class ExecutionContext;
...@@ -39,6 +40,8 @@ class CORE_EXPORT StylePropertyMap : public StylePropertyMapReadOnly { ...@@ -39,6 +40,8 @@ class CORE_EXPORT StylePropertyMap : public StylePropertyMapReadOnly {
StylePropertyMap() = default; StylePropertyMap() = default;
private: private:
bool SetShorthandProperty(const CSSProperty&, const CSSStyleValue&);
DISALLOW_COPY_AND_ASSIGN(StylePropertyMap); DISALLOW_COPY_AND_ASSIGN(StylePropertyMap);
}; };
......
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