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

Issue 1461623002: Disable stretch-to-viewport quirk for multicol. (Closed)

Created:
5 years, 1 month ago by mstensho (USE GERRIT)
Modified:
5 years, 1 month ago
CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, mstensho (USE GERRIT), pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable stretch-to-viewport quirk for multicol. We don't want to stretch the height of BODY to that of the viewport if BODY is inside a multicol container. There should be no need for this quirk in multicol, and even if we had wanted to do something like that, what used to be there was utterly wrong, since we'd end up with the viewport height divided by the number of columns, which is just unpredictable silliness that nobody wants. Un-inline the quirks mode part of stretchesToViewport() into stretchesToViewportInQuirksMode() and check the condition that's most unlikely to be true first, so that we can bail early. R=leviw@chromium.org Committed: https://crrev.com/7ae3919e47ed6b92e1c7c4fc10e58f16a7742f2e Cr-Commit-Position: refs/heads/master@{#360388}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -4 lines) Patch
A third_party/WebKit/LayoutTests/fast/multicol/multicol-on-root-element-quirks.html View 1 chunk +12 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/multicol/multicol-on-root-element-quirks-expected.html View 1 chunk +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.h View 2 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
mstensho (USE GERRIT)
5 years, 1 month ago (2015-11-18 15:46:48 UTC) #1
leviw_travelin_and_unemployed
lgtm I like the patch, but I don't agree with your assertion that no one ...
5 years, 1 month ago (2015-11-18 18:38:11 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1461623002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1461623002/1
5 years, 1 month ago (2015-11-18 18:39:39 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 1 month ago (2015-11-18 19:41:38 UTC) #5
commit-bot: I haz the power
5 years, 1 month ago (2015-11-18 19:43:06 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/7ae3919e47ed6b92e1c7c4fc10e58f16a7742f2e
Cr-Commit-Position: refs/heads/master@{#360388}

Powered by Google App Engine
This is Rietveld 408576698