Bug 139466 – [iOS] Add setting to ignore viewport scaling constraints

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Martin Hock]

[iOS] Add viewport override preference

[Martin Hock]

Created attachment 242982 [details]
patch

[Benjamin Poulain]

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

[Martin Hock]

Created attachment 242985 [details]
address comments

[Benjamin Poulain]

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.

[Martin Hock]

Committed r177110: <http://trac.webkit.org/changeset/177110>