Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Issue 55073002: [Android WebView] Add a quirk to clobber initial scale in certain cases (Closed)

Created:
7 years, 1 month ago by mnaganov (inactive)
Modified:
7 years, 1 month ago
CC:
blink-reviews, jamesr, dglazkov+blink, abarth-chromium, android-webview-reviews_chromium.org
Visibility:
Public.

Description

[Android WebView] Add a quirk to clobber initial scale in certain cases In the following cases, the viewport meta tag would take precedence over the value specified in setInitialPageScaleOverride: The value provided to setInitialPageScaleOverride is <= 1 / device pixel ratio AND - viewport meta has width=device-width OR - viewport meta has auto width, and the page would be displayed at 100% scale. BUG=313754 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161355

Patch Set 1 #

Total comments: 2

Patch Set 2 : Moved under a single if #

Total comments: 2

Patch Set 3 : Fixed checking against device-width #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -2 lines) Patch
M Source/web/PageScaleConstraintsSet.cpp View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M Source/web/WebSettingsImpl.h View 3 chunks +6 lines, -0 lines 0 comments Download
M Source/web/WebSettingsImpl.cpp View 2 chunks +6 lines, -0 lines 0 comments Download
M Source/web/WebViewImpl.cpp View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
M Source/web/tests/WebFrameTest.cpp View 1 2 1 chunk +70 lines, -0 lines 0 comments Download
A Source/web/tests/data/viewport-initial-scale-1.html View 1 chunk +9 lines, -0 lines 0 comments Download
M public/web/WebSettings.h View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
mnaganov (inactive)
Hello, Alexandre! I think, this patch depends on your CL https://codereview.chromium.org/26251010/, are you planning to ...
7 years, 1 month ago (2013-10-31 17:28:06 UTC) #1
aelias_OOO_until_Jul13
Yes, I'll get back to that, sorry for the delay. https://codereview.chromium.org/55073002/diff/1/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/55073002/diff/1/Source/web/WebViewImpl.cpp#newcode2978 ...
7 years, 1 month ago (2013-11-01 00:10:07 UTC) #2
mnaganov (inactive)
https://codereview.chromium.org/55073002/diff/1/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/55073002/diff/1/Source/web/WebViewImpl.cpp#newcode2978 Source/web/WebViewImpl.cpp:2978: if (settingsImpl()->clobberUserAgentInitialScaleQuirk()) { On 2013/11/01 00:10:07, aelias wrote: > ...
7 years, 1 month ago (2013-11-04 17:20:52 UTC) #3
aelias_OOO_until_Jul13
lgtm
7 years, 1 month ago (2013-11-04 17:30:19 UTC) #4
aelias_OOO_until_Jul13
Please see comment below. I think the uses of !isFixed in PageScaleConstraintsSet.cpp should also be ...
7 years, 1 month ago (2013-11-04 17:41:01 UTC) #5
mnaganov (inactive)
https://codereview.chromium.org/55073002/diff/60001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/55073002/diff/60001/Source/web/WebViewImpl.cpp#newcode3012 Source/web/WebViewImpl.cpp:3012: if (!description.maxWidth.isFixed() On 2013/11/04 17:41:01, aelias wrote: > On ...
7 years, 1 month ago (2013-11-04 18:03:16 UTC) #6
mnaganov (inactive)
OK, layout tests also pass (win bot is a fluke). Adam, can you please approve ...
7 years, 1 month ago (2013-11-05 09:31:18 UTC) #7
abarth-chromium
lgtm
7 years, 1 month ago (2013-11-05 15:56:29 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mnaganov@chromium.org/55073002/130001
7 years, 1 month ago (2013-11-05 15:56:34 UTC) #9
commit-bot: I haz the power
7 years, 1 month ago (2013-11-05 17:29:04 UTC) #10
Message was sent while issue was closed.
Change committed as 161355

Powered by Google App Engine
This is Rietveld 408576698