[webkit-reviews] review granted: [Bug 60516] Enable casting between CSSPrimitiveValue and FontWeight enum : [Attachment 92888] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted  review:
Bug 60516: Enable casting between CSSPrimitiveValue and FontWeight enum
https://bugs.webkit.org/show_bug.cgi?id=60516

Attachment 92888: Patch
https://bugs.webkit.org/attachment.cgi?id=92888&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=92888&action=review

>>>>> Source/WebCore/css/CSSStyleSelector.cpp:-3717
>>>>> - 		       return;
>>>> 
>>>> Wait, no, this changes behavior!
>>> 
>>> The old code returned from the function without doing anything if the
identifier was not one of the supported ones. The new code asserts and sets the
weight to normal. I don’t think that behavior change is OK.
>> 
>> I think the default case previously should have been ASSERT_UNREACHABLE.
> 
> Moreover, you can see the list of valid values in the parser:
> // normal | bold | bolder | lighter | 100 | 200 | 300 | 400 | 500 | 600 | 700
| 800 | 900 | inherit
> Any other value would indicate a parser bug.

OK. That makes sense and matches what I remember from other times working with
the parser. I’d feel more comfortable if there was an actual test case covering
this.