[webkit-reviews] review granted: [Bug 61461] REGRESSION: Fullscreen button on embedded Vimeo videos does nothing : [Attachment 94845] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 61461: REGRESSION: Fullscreen button on embedded Vimeo videos does nothing
https://bugs.webkit.org/show_bug.cgi?id=61461

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

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

I think the term “legacy full screen check” here is a little confusing; we
might have been able to find a more specific name that would be clearer.

I am puzzled by all the webkit prefixes on the function names. Those should be
present only for functions exported to the DOM. There’s no point in having
those prefixes in our own internal function names.

> Source/WebCore/dom/Document.h:1062
> +    typedef enum {
> +	   UseStrictFullScreenCheck,
> +	   UseLegacyFullScreenCheck,
> +    } FullScreenCheckType;

The preferred C++ syntax is just:

    enum X { A, B, C };

The syntax you’re using here is C-ism.