Bug 81940 - [chromium] Allow the viewport meta tag to be disabled for testing purposes
: [chromium] Allow the viewport meta tag to be disabled for testing purposes
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 70559
  Show dependency treegraph
 
Reported: 2012-03-22 11:38 PST by
Modified: 2012-03-26 07:16 PST (History)


Attachments
Patch (4.00 KB, patch)
2012-03-22 11:44 PST, Terry Anderson
no flags Review Patch | Details | Formatted Diff | Diff
Patch (4.46 KB, patch)
2012-03-22 12:08 PST, Terry Anderson
no flags Review Patch | Details | Formatted Diff | Diff
Patch for feedback on the placement of viewport functions (4.05 KB, patch)
2012-03-23 10:44 PST, Terry Anderson
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-03-22 11:38:00 PST
Add a new boolean value in WebView to force an early exit of the code in ChromeClientImpl::dispatchViewportPropertiesDidChange even if VIEWPORT is enabled by default.
------- Comment #1 From 2012-03-22 11:44:38 PST -------
Created an attachment (id=133306) [details]
Patch
------- Comment #2 From 2012-03-22 11:53:08 PST -------
(In reply to comment #1)
> Created an attachment (id=133306) [details] [details]
> Patch

please initialize the variable int he constructor of WebViewImpl to false.

Please add a comment in WebView.h
------- Comment #3 From 2012-03-22 12:08:28 PST -------
Created an attachment (id=133313) [details]
Patch
------- Comment #4 From 2012-03-22 12:09:58 PST -------
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.
------- Comment #5 From 2012-03-22 15:27:00 PST -------
(From update of attachment 133313 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=133313&action=review

> Source/WebKit/chromium/public/WebView.h:263
> +    // Viewport -------------------------------------------------------------
> +
> +    // If the viewport is enabled, then the viewport meta tag will allow
> +    // a page to specify its own layout details
> +
> +    virtual bool isViewportEnabled() const = 0;
> +    virtual void enableViewport(bool enable) = 0;

Should this be part of WebPreferences?  That's where we end to put enable flags.
------- Comment #6 From 2012-03-22 15:27:47 PST -------
(From update of attachment 133313 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=133313&action=review

>> Source/WebKit/chromium/public/WebView.h:263
>> +    virtual void enableViewport(bool enable) = 0;
> 
> Should this be part of WebPreferences?  That's where we end to put enable flags.

Sorry, WebSettings: http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/public/WebSettings.h
------- Comment #7 From 2012-03-22 15:28:40 PST -------
(From update of attachment 133313 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=133313&action=review

> Source/WebKit/chromium/public/WebView.h:255
>      virtual WebSize fixedLayoutSize() const = 0;
>      virtual void setFixedLayoutSize(const WebSize&) = 0;

Seems like this should also be a WebSetting...  Maybe there's some reason it's not?
------- Comment #8 From 2012-03-23 10:44:57 PST -------
Created an attachment (id=133509) [details]
Patch for feedback on the placement of viewport functions
------- Comment #9 From 2012-03-23 11:58:43 PST -------
(In reply to comment #7)
> (From update of attachment 133313 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133313&action=review
> 
> > Source/WebKit/chromium/public/WebView.h:255
> >      virtual WebSize fixedLayoutSize() const = 0;
> >      virtual void setFixedLayoutSize(const WebSize&) = 0;
> 
> Seems like this should also be a WebSetting...  Maybe there's some reason it's not?

If these get moved to WebSettings, should

    virtual bool isFixedLayoutModeEnabled() const = 0;
    virtual void enableFixedLayoutMode(bool enable) = 0;

move also or stay in WebView?
------- Comment #10 From 2012-03-23 14:30:41 PST -------
(In reply to comment #9)
> (In reply to comment #7)
> > (From update of attachment 133313 [details] [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=133313&action=review
> > 
> > > Source/WebKit/chromium/public/WebView.h:255
> > >      virtual WebSize fixedLayoutSize() const = 0;
> > >      virtual void setFixedLayoutSize(const WebSize&) = 0;
> > 
> > Seems like this should also be a WebSetting...  Maybe there's some reason it's not?
> 
> If these get moved to WebSettings, should
> 
>     virtual bool isFixedLayoutModeEnabled() const = 0;
>     virtual void enableFixedLayoutMode(bool enable) = 0;
> 
> move also or stay in WebView?

Please leave fixed layout stuff in WebView for now because it touches Page. 

We can clean it up in another patch. Making it setting will require some changes in WebCore.

enableViewport is independent of WebCore.
------- Comment #11 From 2012-03-23 16:26:11 PST -------
(From update of attachment 133509 [details])
We had a pow-wow with fishd, and it sounds like this is the proper place for this setting (since it's scope is a WebView).
------- Comment #12 From 2012-03-26 07:16:26 PST -------
(From update of attachment 133509 [details])
Clearing flags on attachment: 133509

Committed r112088: <http://trac.webkit.org/changeset/112088>
------- Comment #13 From 2012-03-26 07:16:32 PST -------
All reviewed patches have been landed.  Closing bug.