Bug 130152

Summary: Do not show extended background during a pinch gesture on iOS
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, benjamin, jonlee, mitz, sam, simon.fraser, thorton
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
benjamin: review-
Patch simon.fraser: review+

Description Beth Dakin 2014-03-12 12:10:06 PDT
Do not show extended background during a pinch gesture.

<rdar://problem/16303819>
Comment 1 Beth Dakin 2014-03-12 12:14:22 PDT
Created attachment 226543 [details]
Patch
Comment 2 Benjamin Poulain 2014-03-12 12:54:40 PDT
Comment on attachment 226543 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:403
> +    UIColor *pageExtendedBackgroundColor = [self pageExtendedBackgroundColor];
> +    if (pageExtendedBackgroundColor && [self _backgroundExtendsBeyondPage] && [_scrollView zoomScale] >= [_scrollView minimumZoomScale] && ![_scrollView isZoomBouncing])
> +        [_scrollView setBackgroundColor:pageExtendedBackgroundColor];
> +    else
> +        [_scrollView setBackgroundColor:nil];

You'll need the same code executed in response to scrollViewDidZoom:
Comment 3 Simon Fraser (smfr) 2014-03-12 13:03:18 PDT
Comment on attachment 226543 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:403
>> +        [_scrollView setBackgroundColor:nil];
> 
> You'll need the same code executed in response to scrollViewDidZoom:

Yeah, I don't think this is the right place to set the scrollView's background color. I think we should just store the color coming in from the layer commit, then decide where to apply it in response to gestures elsewhere.
Comment 4 Beth Dakin 2014-03-12 13:32:50 PDT
(In reply to comment #3)
> (From update of attachment 226543 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226543&action=review
> 
> >> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:403
> >> +        [_scrollView setBackgroundColor:nil];
> > 
> > You'll need the same code executed in response to scrollViewDidZoom:
> 
> Yeah, I don't think this is the right place to set the scrollView's background color. I think we should just store the color coming in from the layer commit, then decide where to apply it in response to gestures elsewhere.

Just so you know, we do already cache the color. We cache it in WebPageProxy::didCommitLayerTree.

New patch forthcoming!
Comment 5 Beth Dakin 2014-03-12 13:36:13 PDT
Created attachment 226548 [details]
Patch
Comment 6 mitz 2014-03-13 11:06:21 PDT
Comment on attachment 226548 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:388
> +- (void)updateScrollViewBackground

This internal method’s name should have an underscore prefix.
Comment 7 Beth Dakin 2014-03-13 11:46:43 PDT
(In reply to comment #6)
> (From update of attachment 226548 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226548&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:388
> > +- (void)updateScrollViewBackground
> 
> This internal method’s name should have an underscore prefix.

Done!
Comment 8 Beth Dakin 2014-03-13 15:45:59 PDT
http://trac.webkit.org/changeset/165573