Bug 81940

Summary: [chromium] Allow the viewport meta tag to be disabled for testing purposes
Product: WebKit Reporter: Terry Anderson <tdanderson>
Component: Tools / TestsAssignee: 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 Flags
Patch
none
Patch
none
Patch for feedback on the placement of viewport functions none

Terry Anderson
Reported 2012-03-22 11:38:00 PDT
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.
Attachments
Patch (4.00 KB, patch)
2012-03-22 11:44 PDT, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Patch (4.46 KB, patch)
2012-03-22 12:08 PDT, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Patch for feedback on the placement of viewport functions (4.05 KB, patch)
2012-03-23 10:44 PDT, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Terry Anderson
Comment 1 2012-03-22 11:44:38 PDT
Created attachment 133306 [details] Patch
Fady Samuel
Comment 2 2012-03-22 11:53:08 PDT
(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
Terry Anderson
Comment 3 2012-03-22 12:08:28 PDT
Created attachment 133313 [details] Patch
WebKit Review Bot
Comment 4 2012-03-22 12:09:58 PDT
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.
Adam Barth
Comment 5 2012-03-22 15:27:00 PDT
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.
Adam Barth
Comment 6 2012-03-22 15:27:47 PDT
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
Adam Barth
Comment 7 2012-03-22 15:28:40 PDT
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?
Terry Anderson
Comment 8 2012-03-23 10:44:57 PDT
Created attachment 133509 [details] Patch for feedback on the placement of viewport functions
Terry Anderson
Comment 9 2012-03-23 11:58:43 PDT
(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?
Fady Samuel
Comment 10 2012-03-23 14:30:41 PDT
(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.
Adam Barth
Comment 11 2012-03-23 16:26:11 PDT
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).
WebKit Review Bot
Comment 12 2012-03-26 07:16:26 PDT
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>
WebKit Review Bot
Comment 13 2012-03-26 07:16:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.