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

Issue 699253002: Fixing two layout tests on Android broken by enabling virtual-viewport (Closed)

Created:
6 years, 1 month ago by bokan
Modified:
6 years, 1 month ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Fixing two layout tests on Android broken by enabling virtual-viewport The svg test turned out to simply need a rebaseline as the scrollbars should no longer show up in layout tests. The autoresize test was off by 1 pixel. This was happening because in virtual viewport mode we size the main FrameView's width to the content size and the height by the aspect ratio. This meant that the height would have some tiny error in some aspect ratios which would get blown up to a full pixel when we returned an expandedIntSize in PageScaleConstraintsSet::mainFrameSize. This is fixed by explicitly setting the FrameView size equal to the viewport size if the content width matches the viewport width. BUG=428189 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184840

Patch Set 1 #

Patch Set 2 : Use viewportHeight if contentWidth == viewportWidth #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -3 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +1 line, -3 lines 0 comments Download
M Source/web/PageScaleConstraintsSet.cpp View 1 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
bokan
ptal
6 years, 1 month ago (2014-11-04 20:24:56 UTC) #2
aelias_OOO_until_Jul13
On 2014/11/04 20:24:56, bokan wrote: > ptal Could you instead change the aspect ratio code ...
6 years, 1 month ago (2014-11-04 20:33:49 UTC) #3
aelias_OOO_until_Jul13
On second thought, it would be cleaner to bypass the aspect ratio and use viewportHeight ...
6 years, 1 month ago (2014-11-04 20:36:55 UTC) #4
bokan
On 2014/11/04 20:36:55, aelias wrote: > On second thought, it would be cleaner to bypass ...
6 years, 1 month ago (2014-11-04 20:54:23 UTC) #5
aelias_OOO_until_Jul13
lgtm
6 years, 1 month ago (2014-11-04 20:55:00 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/699253002/20001
6 years, 1 month ago (2014-11-04 20:57:59 UTC) #8
commit-bot: I haz the power
6 years, 1 month ago (2014-11-04 22:24:37 UTC) #9
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as 184840

Powered by Google App Engine
This is Rietveld 408576698