[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