[webkit-reviews] review granted: [Bug 205901] Supplying CSS global keywords to the Font Loading API can cause crashes : [Attachment 387062] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 205901: Supplying CSS global keywords to the Font Loading API can cause
crashes
https://bugs.webkit.org/show_bug.cgi?id=205901

Attachment 387062: Patch

https://bugs.webkit.org/attachment.cgi?id=387062&action=review




--- Comment #3 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 387062
  --> https://bugs.webkit.org/attachment.cgi?id=387062
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387062&action=review

> Source/WebCore/ChangeLog:3
> +	   Supplying CSS global keywords to the Font Loading API can cause
crashes

Suggest changing the title to something like:

"Fix specification violation in Font Loading API"

> Source/WebCore/ChangeLog:9
> +	   Simply add a check. The spec says:

Then say: "Our Font Loading API does not comply with the spec: <<< spec
language >>>"

"Rather than crashing, we should return a syntax error."

> LayoutTests/fast/text/font-loading-global-keyword-expected.txt:1
> +This test passes if there is no crash.

I don't think this is true. The test passes if we get the expected syntax
errors. No need to say something about a crash (all of our tests are expected
to pass without crashing!)

> LayoutTests/fast/text/font-loading-global-keyword.html:8
> +description("This test passes if there is no crash.");

description("This test ensure that global CSS keywords generate expected syntax
errors.");