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

Issue 809193002: Don't apply column-span:all for display:table-* (Closed)

Created:
6 years ago by mstensho (USE GERRIT)
Modified:
6 years ago
Reviewers:
dsinclair
CC:
blink-reviews, blink-reviews-rendering, zoltan1, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, Dominik Röttsches, jchaffraix+rendering
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Don't apply column-span:all for display:table-* The spec doesn't allow this, and we crash if we allow it. This fix is specific to the old / current multicol implementation. The new one already does it correctly, FWIW. The old / current multicol implementation has this concept of block continuations, which we establish when there's a spanner inside some multicol child. We do this before considering any render tree fixup for anonymous table objects. These two render tree fixup mechanisms simply don't play well together, and since table-* cannot become spanners, according to the spec, there's no need to make them cope with such situations. Just prevent spanners in such cases. BUG=435815, 443205 R=dsinclair@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187422

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+188 lines, -1 line) Patch
A LayoutTests/fast/multicol/span/table-caption-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-caption-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-cell-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-cell-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-column-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-column-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-column-group-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-column-group-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-row-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-row-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-row-group-crash.html View 1 chunk +29 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/span/table-row-group-crash-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBlock.cpp View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
mstensho (USE GERRIT)
6 years ago (2014-12-17 23:16:59 UTC) #1
dsinclair
lgtm
6 years ago (2014-12-18 01:11:23 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/809193002/1
6 years ago (2014-12-18 01:13:09 UTC) #4
commit-bot: I haz the power
6 years ago (2014-12-18 02:42:29 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187422

Powered by Google App Engine
This is Rietveld 408576698