Summary: | [chromium] Allow the viewport meta tag to be disabled for testing purposes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Terry Anderson <tdanderson> | ||||||||
Component: | Tools / Tests | Assignee: | Terry Anderson <tdanderson> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, dglazkov, fishd, fsamuel, jamesr, rjkroege, tkent, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 70559 | ||||||||||
Attachments: |
|
Description
Terry Anderson
2012-03-22 11:38:00 PDT
Created attachment 133306 [details]
Patch
(In reply to comment #1) > Created an attachment (id=133306) [details] > Patch please initialize the variable int he constructor of WebViewImpl to false. Please add a comment in WebView.h Created attachment 133313 [details]
Patch
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 on attachment 133313 [details] Patch 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 on attachment 133313 [details] Patch 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 on attachment 133313 [details] Patch 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? Created attachment 133509 [details]
Patch for feedback on the placement of viewport functions
(In reply to comment #7) > (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? If these get moved to WebSettings, should virtual bool isFixedLayoutModeEnabled() const = 0; virtual void enableFixedLayoutMode(bool enable) = 0; move also or stay in WebView? (In reply to comment #9) > (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? 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 on attachment 133509 [details]
Patch for feedback on the placement of viewport functions
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 on attachment 133509 [details] Patch for feedback on the placement of viewport functions Clearing flags on attachment: 133509 Committed r112088: <http://trac.webkit.org/changeset/112088> All reviewed patches have been landed. Closing bug. |