Expand the "font" shorthand when specified using a system font keyword.

Expand the "font" shorthand when specified using a system font keyword.

R=darktears
BUG=331801

[apavlov, 2014-01-30 16:35:35]

The problem with the patch is:

style.font = "-webkit-control";
style.font = "";

style.font == "-webkit-control";

Do you have a clue on how to fix this best?

[apavlov, 2014-02-04 08:10:17]

The problem is obviously due to the way shorthand properties are removed: only
longhands are, not the shorthand proper. However I need to add the "font"
shorthand explicitly to the style, since otherwise it will not be present in the
declaration altogether, since all of its longhands will be implicit. I'd
appreciate any hints on the solution.

[apavlov, 2014-02-14 12:41:26]

ping?

[apavlov, 2014-03-14 08:27:28]

Friendly ping

[darktears, 2014-03-18 12:12:36]

On 2014/01/30 16:35:35, apavlov wrote:
> The problem with the patch is:
> 
> style.font = "-webkit-control";
> style.font = "";
> 
> style.font == "-webkit-control";
> 
> Do you have a clue on how to fix this best?

bool MutableStylePropertySet::removeShorthandProperty(CSSPropertyID propertyID)
is probably not working the way you want it to work here. It will try to match
font as a shorthand, will not find its longhand so the property is not going to
be removed.

[apavlov, 2014-03-20 08:20:00]

Patch updated per Tab's response.

[darktears, 2014-03-20 13:20:17]

Thanks for doing this. LGTM.

https://codereview.chromium.org/150333003/diff/80001/Source/core/css/parser/C...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/150333003/diff/80001/Source/core/css/parser/C...
Source/core/css/parser/CSSPropertyParser.cpp:4529: return false;
nit : You could assert here, so that we know if we enter there with several
words in the valueList, we have a problem.

[apavlov, 2014-03-20 14:38:24]

https://codereview.chromium.org/150333003/diff/80001/Source/core/css/parser/C...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/150333003/diff/80001/Source/core/css/parser/C...
Source/core/css/parser/CSSPropertyParser.cpp:4529: return false;
On 2014/03/20 13:20:17, darktears wrote:
> nit : You could assert here, so that we know if we enter there with several
> words in the valueList, we have a problem.

Done.

[apavlov, 2014-03-20 14:38:34]

The CQ bit was checked by apavlov@chromium.org

[commit-bot: I haz the power, 2014-03-20 14:38:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apavlov@chromium.org/150333003/100001

[commit-bot: I haz the power, 2014-03-20 14:41:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 14:41:30]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_rel

[apavlov, 2014-03-20 14:48:29]

The CQ bit was unchecked by apavlov@chromium.org

[apavlov, 2014-03-20 15:53:07]

Closed, since https://codereview.chromium.org/205743004/ does the same thing
(and a tad more)