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

Issue 1292163002: Initial support for nested multicol layout. (Closed)

Created:
5 years, 4 months ago by mstensho (USE GERRIT)
Modified:
5 years, 4 months ago
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

Initial support for nested multicol layout. Nested column balancing doesn't work yet. Also no support for mapping visual points to flow thread points (which is used by a few operations, but not by the most common ones, like painting and hit-testing). There are other corner-cases to sort out, too. Still no support for printing multicol documents, but that's the ultimate goal of this work. BUG=447718 R=jchaffraix@chromium.org,leviw@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200639

Patch Set 1 #

Total comments: 4

Patch Set 2 : code review #

Messages

Total messages: 11 (2 generated)
mstensho (USE GERRIT)
5 years, 4 months ago (2015-08-13 20:14:59 UTC) #1
leviw_travelin_and_unemployed
I probably won't get to this until tomorrow, but looks exciting! Is there by any ...
5 years, 4 months ago (2015-08-13 20:17:16 UTC) #2
mstensho (USE GERRIT)
On 2015/08/13 20:17:16, leviw wrote: > I probably won't get to this until tomorrow, but ...
5 years, 4 months ago (2015-08-13 20:47:10 UTC) #3
leviw_travelin_and_unemployed
LGTM. Sorry, it took me awhile to wrap my head around all of this. Do ...
5 years, 4 months ago (2015-08-15 00:04:43 UTC) #4
mstensho (USE GERRIT)
https://codereview.chromium.org/1292163002/diff/1/Source/core/layout/LayoutMultiColumnSet.cpp File Source/core/layout/LayoutMultiColumnSet.cpp (right): https://codereview.chromium.org/1292163002/diff/1/Source/core/layout/LayoutMultiColumnSet.cpp#newcode121 Source/core/layout/LayoutMultiColumnSet.cpp:121: { // extra scope here for previousGroup; it's potentially ...
5 years, 4 months ago (2015-08-17 07:43:09 UTC) #5
mstensho (USE GERRIT)
On 2015/08/15 00:04:43, leviw wrote: > LGTM. > > Sorry, it took me awhile to ...
5 years, 4 months ago (2015-08-17 07:44:18 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1292163002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1292163002/20001
5 years, 4 months ago (2015-08-17 07:44:34 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=200639
5 years, 4 months ago (2015-08-17 10:12:16 UTC) #10
leviw_travelin_and_unemployed
5 years, 4 months ago (2015-08-17 20:00:17 UTC) #11
Message was sent while issue was closed.
On 2015/08/17 at 07:44:18, mstensho wrote:
> On 2015/08/15 00:04:43, leviw wrote:
> > LGTM.
> > 
> > Sorry, it took me awhile to wrap my head around all of this.
> 
> More than fast enough. Thanks!
> 
> > Do we have intentions of importing the w3c tests for this?
> 
> I don't know. What's the general practice for things like this? Should I steal
(and adapt, i.e. prefix) some tests and create a fast/multicol/imported/ ?

I'm mostly just interested in the test coverage/compatibility. If you think
they're useful, you should go ahead and import them with prefixes. If you don't,
don't bother. Just curious if you were using it as a guide on what to fix.

> 
> > https://www.chromium.org/developers/design-documents/multi-column-layout
could
> > probably also use an update.
> 
> Absolutely. I'll write something there soon.

Awesome!

Powered by Google App Engine
This is Rietveld 408576698