[webkit-reviews] review granted: [Bug 186862] [Fullscreen] Use secondary glyph style for fullscreen controls : [Attachment 343189] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 20 23:59:29 PDT 2018


Tim Horton <thorton at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 186862: [Fullscreen] Use secondary glyph style for fullscreen controls
https://bugs.webkit.org/show_bug.cgi?id=186862

Attachment 343189: Patch

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




--- Comment #4 from Tim Horton <thorton at apple.com> ---
Comment on attachment 343189
  --> https://bugs.webkit.org/attachment.cgi?id=343189
Patch

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

> Source/WebKit/UIProcess/ios/fullscreen/WKFullscreenStackView.mm:51
> +    CGRect frame = CGRectMake(0, 0, 100, 100);

Where does this 100 come from?

> Source/WebKit/UIProcess/ios/fullscreen/WKFullscreenStackView.mm:59
> +    _backgroundView = [allocAVBackgroundViewInstance() initWithFrame:frame];

Why does this not sit in a RetainPtr?

> Source/WebKit/UIProcess/ios/fullscreen/WKFullscreenStackView.mm:71
> +- (void)addArrangedSubview:(UIView *)subview
applyingMaterialStyle:(AVBackgroundViewMaterialStyle)materialStyle
tintEffectStyle:(AVBackgroundViewTintEffectStyle)tintEffectStyle

Probably you want a leading underscore here? Or is this an override of a real
thing? Seems not to be.


More information about the webkit-reviews mailing list