Bug 130152 - Do not show extended background during a pinch gesture on iOS
Summary: Do not show extended background during a pinch gesture on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-12 12:10 PDT by Beth Dakin
Modified: 2014-03-13 15:45 PDT (History)
7 users (show)

See Also:

Attachments
Patch (2.20 KB, patch)
2014-03-12 12:14 PDT, Beth Dakin 		benjamin: review-
Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2014-03-12 13:36 PDT, Beth Dakin 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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