[Chromium] Let the embedder override the max page scale factor set by the page
WebView.h: // PageScaleFactor will be force-clamped between minPageScale and maxPageScale // (and these values will persist until setPageScaleFactorLimits is called // again). virtual void setPageScaleFactorLimits(float minPageScale, float maxPageScale) = 0; is that not sufficient?
Created attachment 148217 [details] Patch
> is that not sufficient? No, that's a ceiling for the max page scale factor. This patch lets the embedder set a floor for the max page scale factor.
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
@jamesr: What's the next step here? Are you happy with the design if the API, or do we need to iterate? I've verified that this feature is actively used in chromium-android. (In Chrome for Android, if you go into Settings -> Accessibility, this code implements the "Force enable zoom" checkbox.)
FYI, setPageScaleFactorLimits is no longer used by Chrome for Android (because it was for viewport tag logic now moved into WebViewImpl), nor are there plans to use it again in the future. The only user is now on desktop platforms to disable pinch-zoom. We should probably remove it, especially since the setting is liable to be unexpectedly overridden by the viewport tag anyway. Anyway, as for this value, I think passing a boolean would be clearer, there are already confusingly many min/max values and I can't think of a reason the embedder would want to pass a specific number. I suggest naming it "ignoreViewportTagMaximumScale".
(In reply to comment #6) > FYI, setPageScaleFactorLimits is no longer used by Chrome for Android (because it was for viewport tag logic now moved into WebViewImpl), nor are there plans to use it again in the future. The only user is now on desktop platforms to disable pinch-zoom. We should probably remove it, especially since the setting is liable to be unexpectedly overridden by the viewport tag anyway. Ok. I'll investigate and prepare a patch to remove it. > Anyway, as for this value, I think passing a boolean would be clearer, there are already confusingly many min/max values and I can't think of a reason the embedder would want to pass a specific number. I suggest naming it "ignoreViewportTagMaximumScale". The implementation in the chromium-android branch uses a boolean, but then there's this funny 5.0f constant. I guess that constant needs to be somewhere.
For the number, we can use: const float WebView::maxPageScaleFactor = 4.0; which is defined in WebViewImpl and already exported in WebView. (The exact number 5 is not important, 4 is fine.)
Comment on attachment 148217 [details] Patch Ok. Will do. By the way, I think 4.0 was the real limit in practice in the chromium-android branch too.
Created attachment 149367 [details] work in progress
Created attachment 149379 [details] Patch
> > FYI, setPageScaleFactorLimits is no longer used by Chrome for Android (because it was for viewport tag logic now moved into WebViewImpl), nor are there plans to use it again in the future. The only user is now on desktop platforms to disable pinch-zoom. We should probably remove it, especially since the setting is liable to be unexpectedly overridden by the viewport tag anyway. > > Ok. I'll investigate and prepare a patch to remove it. Turns out setPageScaleFactorLimits is still used by WebKit internally to implement the viewport meta tag. We might still want to remove the API given that it's not clear that the page can override it using a meta tag.
Created attachment 149381 [details] Patch
Comment on attachment 149381 [details] Patch R=me, looks fine.
Comment on attachment 149381 [details] Patch Thanks!
Comment on attachment 149381 [details] Patch Clearing flags on attachment: 149381 Committed r121213: <http://trac.webkit.org/changeset/121213>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) > > > FYI, setPageScaleFactorLimits is no longer used by Chrome for Android (because it was for viewport tag logic now moved into WebViewImpl), nor are there plans to use it again in the future. The only user is now on desktop platforms to disable pinch-zoom. We should probably remove it, especially since the setting is liable to be unexpectedly overridden by the viewport tag anyway. > > > > Ok. I'll investigate and prepare a patch to remove it. > > Turns out setPageScaleFactorLimits is still used by WebKit internally to implement the viewport meta tag. We might still want to remove the API given that it's not clear that the page can override it using a meta tag. Right, sorry I wasn't clear, I only meant remove the API from public WebView.