Bug 139466

Summary: [iOS] Add setting to ignore viewport scaling constraints
Product: WebKit Reporter: Martin Hock <mhock>
Component: WebCore Misc.Assignee: Martin Hock <mhock>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, mhock
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
benjamin: review-
address comments benjamin: review+, benjamin: commit-queue-

Description Martin Hock 2014-12-09 17:24:53 PST
[iOS] Add viewport override preference
Comment 1 Martin Hock 2014-12-09 17:39:16 PST
Created attachment 242982 [details]
patch
Comment 2 Benjamin Poulain 2014-12-09 17:56:00 PST
Comment on attachment 242982 [details]
patch

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

The name "Override" is too generic. What do you think of "ignoreViewportScalingConstraints"?

> Source/WebCore/page/ViewportConfiguration.cpp:241
> -    applyViewportArgument(m_configuration.minimumScale, m_viewportArguments.minZoom, minimumViewportArgumentsScaleFactor, maximumViewportArgumentsScaleFactor);
> +    applyViewportArgument(m_configuration.minimumScale, m_override ? minimumViewportArgumentsScaleFactor : m_viewportArguments.minZoom, minimumViewportArgumentsScaleFactor, maximumViewportArgumentsScaleFactor);

I think I would leave the configuration unchanged, and modify minimumScale() instead.

That will make it easier to reason about the viewport configuration. The state of m_configuration would still be correct, but we would report different values to the API.

> Source/WebCore/page/ViewportConfiguration.cpp:260
> +    if (m_override)
> +        m_configuration.allowsUserScaling = true;

ditto
Comment 3 Martin Hock 2014-12-09 20:21:03 PST
Created attachment 242985 [details]
address comments
Comment 4 Benjamin Poulain 2014-12-09 20:27:19 PST
Comment on attachment 242985 [details]
address comments

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

> Source/WebCore/page/ViewportConfiguration.h:80
> +    void setIgnoreViewportScalingConstraints(bool ignoreViewportScalingConstraints) {m_ignoreViewportScalingConstraints = ignoreViewportScalingConstraints; }

I wonder if for Viewport Configuration, this should be named "IgnoreScalingConstraints" since the Viewport is implied by the object.

I let you decide the name.

> Source/WebCore/page/ViewportConfiguration.h:111
> +    bool m_ignoreViewportScalingConstraints;

You should initialize this in the constructor.
Comment 5 Martin Hock 2014-12-10 15:42:11 PST
Committed r177110: <http://trac.webkit.org/changeset/177110>