Summary: | Do not show extended background during a pinch gesture on iOS | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||
Component: | WebKit2 | Assignee: | 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
Beth Dakin
2014-03-12 12:10:06 PDT
Created attachment 226543 [details]
Patch
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 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. (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! Created attachment 226548 [details]
Patch
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. (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! |