Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-multicol] column-fill should behave more similarly in paginated and continuous contexts #4036

Closed
dbaron opened this issue Jun 15, 2019 · 13 comments
Labels
css-multicol-1 Current Work

Comments

@dbaron
Copy link
Member

dbaron commented Jun 15, 2019

Following the fix for #3224, the behavior of column-fill appears to be unnecessarily different between paginated and continuous contexts.

I would suggest fixing this asymmetry by changing the sentence that begins:

In continuous contexts, this property will only be consulted if the length of columns has been constrained in the block dimension, ...

to instead begin:

In continuous contexts, and for the last fragment in fragmented contexts, this property will only be consulted if the length of columns has been constrained in the block dimension, ...

This change will make the behavior for the last fragment in a fragmented context match the behavior in a continuous context, which I think is a good thing. This makes layouts more consistent when they're transferred between fragmented and unfragmented contexts; e.g., a web page changes less as a result of being printed.

In other words, what I'm suggesting is that balancing would be determined by this table (where yes/no is whether to balance), with the * indicating the cases I propose to change:

value length fragmented context, not last fragment fragmented context, last fragment continuous context
auto any no no no
balance constrained no yes yes
balance unconstrained no no* no
balance-all constrained yes yes yes
balance-all unconstrained yes no* no
@dbaron dbaron added the css-multicol-1 Current Work label Jun 15, 2019
@rachelandrew rachelandrew added this to Unfiled in css-multicol-1 Jun 16, 2019
@rachelandrew rachelandrew moved this from Unfiled to Needs discussion in css-multicol-1 Jun 16, 2019
@mstensho
Copy link

Your suggestion seems fine.

However, I have a hard time understanding how that would result in such a table.

The first row: column-fill:auto should force balancing in a continuous context, unless block-size is constrained. So you probably need to split the first row in two, one for constrained length (no balancing no matter what), and one for unconstrained length (balance in continuous media). That would represent the reality of today.

Furthermore, balance-all, unconstrained block-size, and last fragmentainer (last row, second-to-last cell): This one should say "yes", shouldn't it? We definitely want to balance if balance-all is specified. Always. So the last cell in the last row should also say "yes".

The "no*" in the third row doesn't seem right, either. If height is unconstrained, we should balance the last fragmentainer. I think you want to say "yes*".

I'm imagining this:
If column-fill is balance-all: Always balance, everywhere.
If column-fill is balance: Always balance, except when participating in an outer fragmentation context and it is not the last fragmentainer.
If column-fill is auto: Never balance, unless block-size is unconstrained. Even if block size is unconstrained, do not balance if we're participating in an outer fragmentation context and it is not the last fragmentainer.

@dbaron
Copy link
Member Author

dbaron commented Jun 19, 2019

Yeah, you're right, I had the table wrong; I was working too quickly and got the defaults backwards. I guess the table should be:

value length fragmented context, not last fragment fragmented context, last fragment continuous context
auto constrained no no no
auto unconstrained no yes* yes
balance any no yes yes
balance-all any yes yes yes

If you agree this is correct, then maybe instead of only making the wording change I propose, it would be better to also move that extra paragraph into the definition of auto and reword appropriately.

@mstensho
Copy link

Yeah, you're right, I had the table wrong; I was working too quickly and got the defaults backwards. I guess the table should be:

Yes, that looks right to me!

If you agree this is correct, then maybe instead of only making the wording change I propose, it would be better to also move that extra paragraph into the definition of auto and reword appropriately.

That's also a very good idea.

@dbaron
Copy link
Member Author

dbaron commented Jun 19, 2019

I also just wrote some examples to look at browser behavior. The examples aren't even useful on Gecko or WebKit due to other bugs; it appears that Chromium would need to change behavior if we make this change.

@mstensho
Copy link

Yes, Chromium still doesn't consider min-height or max-height when determining constrainedness. There's https://bugs.chromium.org/p/chromium/issues/detail?id=967329 , which was reported for the max-height case. I'll change that bug to also cover the min-height case.

Also note that Chromium doesn't recognize the "balance-all" value at all. That's https://bugs.chromium.org/p/chromium/issues/detail?id=909596

Thanks.

@frivoal
Copy link
Collaborator

frivoal commented Jun 26, 2019

I sympathize with @dbaron 's request for a cleaner system and symmetry between the continuous and fragmented context.

However, there's a use both for balancing and not balancing the last page even when the height is not constrained. We could change the behavior as suggested and add the missing one later as an extra keyword, but multicol in fragmented media is routinely used in print and pdf formatters, and they implement the spec as it currently is, balancing the columns on the last page even with no height constrain when column-fill is balance, and not balancing when column-fill is auto. Given that, this class of user agents are likely to have a compat requirement to keep this behavior, and won't change. I think it would be very unfortunate to fork the language, and for print formatters and browsers to have different behavior on multi-column.

(I have tested Prince and PDFreactor and they are interoperable. Vivliostyle has some bugs, but for that aspect it is interoperable as well.)

All in all, I think we should reject the proposal.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed column-fill should behave more similarly in paginated and continuous contexts.

The full IRC log of that discussion <dael> Topic: column-fill should behave more similarly in paginated and continuous contexts
<dael> github: https://github.com//issues/4036
<dael> Rossen_: Opened by dbaron . Is he on?
<dael> [silence]
<dael> Rossen_: Can anyone take or should we wait?
<dael> florian: I can
<dael> florian: There is currently a difference between what happens to columns in fragmeneted and non-frag context and there's no contraint on height
<dael> florian: dbaron proposed to align frag context to non-frag
<dael> florian: My reason why not is that the behavior spec in frag context is the one pdf and print formatters impl interop. multicol in frag documents is used a lot.
<dael> florian: If there wasn't a compat concern we could align but in print we do have a dependency. I don't think they'll change and I don't want to fork language. Based on that I suggest don't
<dael> dbaron: Important diff is in browsers people care about print being similar to on screen. If we leave this we end up with for multicols that happen to be in frag context and don't frag or for the last frag of a multicol in frag context you get different balancing then on screen. Problematic. Most for when you print and it's not frag and balancing on paper is different.
<fantasai> So basically we have two different, conflicting compat constraints. :/
<dael> Rossen_: dbaron to visualize I'm assuming talking about multicol that has auto balance and in the continuous space you have plenty of height so no balance
<dael> dbaron: yeah
<dael> Rossen_: In print when we reach the last frag where the column would otherwise frag to one per page we're not balancing per frag we'll have 1 column per frag
<dael> dbaron: Not sure what you mean. Confused me with last bit
<florian> this table has the summary: https://github.com//issues/4036#issuecomment-503648397
<dael> Rossen_: When we go to print and we have fragment now they have a constrained height which triggers balancing
<dael> Rossen_: Or did I mis-read?
<AmeliaBR> Is “make browsers match print behavior” (for the last fragment OR for no fragments) still an option?
<dael> Rossen_: What is different between continuous medium and fragmented
<dael> florian: 2 tables in GH, 2nd is correct. unconstrained frag with auto the columns don't balance.
<dael> florian: Would switch to balance columns when column fill is auto in an unconstrained context. Can't have unbalanced column on last page
<dael> Rossen_: Balancing on last page in my case, it's only on last page with current prop?
<dael> florian: All pages including last.
<dael> florian: Currently it's all except last (I think)
<fantasai> I think the mistake in multicol was that continuous media balances in cases where print does not, but I guess that's not fixable...
<dael> dbaron: With column-fill:auto and unconstrained height you do not balance on any page in frag context. In continuous you balance
<dael> florian: Okay. I missed a step. I think print formatters balance on every page but the last.
<dael> Rossen_: That's...interesting. Almost opposite of proposal
<dael> florian: Proposal assumes spec says on non-last pages don't balance. That's not what print impl do. They don't balance on last page but do on others. Let me check spec
<dael> Rossen_: Reason I looked at table as misleading you said for auto balancing unconstrained if you're in frag and not last you won't balance. If you're in frag context and last you will balance. Continuous you always balance. Right dbaron ?
<dael> dbaron: In continuous you don't balance with auto and constrained.
<dael> Rossen_: Unconstrained case. When you're in frag context but not last frag you don't balance
<dael> florian: That's dbaron table but spec says when value is auto you fill sequentially
<dael> dbaron: Let me open spec. I think there's words. Not in value def.
<dael> Rossen_: Sounds like in this cae we print 4 pages with one column and on last page we have 3 col and that's weird. We either always or never balance. Half and half is weird.
<dael> dbaron: I think the previous resolution was also somewhat odd. Reverting previous resolution gets us consistent state
<dael> Rossen_: [missed]
<dael> florian: Which dbaron ?
<dael> Rossen_: 3224?
<dael> dbaron: Yes
<Rossen_> https://github.com//issues/3224
<dael> Rossen_: This one ^
<dael> [everyone reads]
<dael> dbaron: Regarding florian question on auto, I agree it's not clear for unblanaced. Clear for balance they are not. Balance says [reads]
<dael> florian: What that means is not single col is filled, all filled. but if you honor force break they have different heights. Means you fill sequentially
<dael> dbaron: That's what I mean by not balanced in my table
<dael> florian: Except for last frag where not balance means fill first column and stop
<dael> dbaron: With auto height?
<dael> florian: Yes
<dael> dbaron: Messy with auto height where have to define auto height calc which can vary
<dael> florian: There's two behaviors for the no. Fill multiple and no balance or fill a single column
<dael> dbaron: At a high level you might see those, but it's a result of what auto-height calculation does
<dael> florian: What we'd lose with your prop is ability on last page to only fill first column. Or do you not lose that?
<dael> florian: If you have mutli page and last has only enough to fill 2/3 of one column printers have 1 column and your prop there is 3
<dael> dbaron: Correct. Changed for continuous already in previous resolution.
<dael> dbaron: There are cases where mutlicol is relatively small. You may well print and it fits on one page
<dael> dbaron: So if you're in this case where previous resolution effects and it fits on one page previous resolution only applies before you print. Afte ryou print we do completely the opposite. That's a major inconsistancy
<dael> florian: True. Two problems. browser printing vs printing and pdf printing vs printing
<dael> florian: What will save us if you called for auto it's okay and [missed]
<dael> dbaron: Reality of printing in browsers users care about results even if authors didn't
<dael> fantasai: I think florian says we've got conflicting compat constraints
<dael> dbaron: 3
<dael> fantasai: Ones I"m aware of is printing through pdf formatter and printing through abrowser should have same result. Other is printing through a browser should have same result as looking at page without printing.
<dael> dbaron: 2 web compat. One is PDF. Other is what led to 3224 which is Chromium and Webkit did one thing and Gecko the other.
<dael> fantasai: So that's a 3rd
<dael> fantasai: What florian pointed out is because auto is not initial it's possible 3rd between Chromium and others is not incompat and we can revert
<dael> florian: THat's possible. What I was saying is because balance is initial most browsers will be on balance and when you print it gets you the same thing. Don't change balance. For auto behavior changes only if you haven't cosntrained. If you constrained auto and balance do the same. If they do both or neither we're safe.
<dael> dbaron: People leave things in code that didn't work
<dael> florian: True. I agree revert prevoius if we can. That's only sane way. Would require WK and Chromium to change
<dael> Rossen_: Opinions from WebKit or Chromium impl?
<dael> [silence]
<dael> dbaron: I think we could cycle back. I don't know if Morten is on but he seems to know about Chromium limitation. There was new information an hour before the call
<dael> florian: Sorry, jsut finished compat testing
<dael> dbaron: There's a new proposal, should let it cycle on the issue and come back.
<dael> fantasai: Anyone else on the call with a concern or are we proposing what florian and dbaron say?
<dael> Rossen_: Proposal is revert resolution on issue #3224. With that we leave discussion open for a week. When others can catch up we can re-open the issue and try to resolve.
<dael> florian: sgtm

@dbaron
Copy link
Member Author

dbaron commented Jun 27, 2019

A few points I'd like to highlight from the discussion in the teleconference yesterday:

  • @frivoal raised the issue that changing this as I initially proposed would be inconsistent with the behavior of print formatters
  • [css-multicol] Improve column-fill and make it backward-compatible #3224 was originally resolving an issue where today's browsers behave differently (Gecko on one side, Blink and WebKit (and EdgeHTML) on the other side), but we weren't aware (I think) of compatibility problems caused by that difference
  • my concern in this issue was inconsistency between formatting of multicol on screen and when printed; users printing from web browsers expect the printing to produce something generally similar to what they see on screen (and the developers often haven't considered or tested printing behavior). My underlying principle is that the last fragment in a paginated context should behave like the behavior in a continuous context; this is particularly important because the last fragment is often the only fragment, and it is bad for something that is not actually fragmented to behave in a substantially different way on paper.

So the option that seemed the most attractive in the teleconference was to revert the change in #3224 and ask Blink and WebKit to to change their behavior.

At the same time... we should probably also clarify what the height: auto behavior is in this case. What do browsers do, and what do print formatters do?

(I think reverting the decision also avoids having to figure out which cases where the height is influenced by the flex or grid algorithms count as a constrained height and which ones don't.)

@dbaron
Copy link
Member Author

dbaron commented Jul 3, 2019

To be extra clear: I'd like to get feedback from Blink and WebKit implementors about the previous comment, and whether they're willing to implement it.

@astearns
Copy link
Member

astearns commented Jul 3, 2019

@mstensho @hober Any comment on this so we can make progress?

@astearns astearns removed the Agenda+ label Jul 3, 2019
@mstensho
Copy link

mstensho commented Jul 4, 2019

You're asking if I'm okay with #3224 being reverted and willing to implement it in Blink?
Sure, but then we won't have to clarify what column-fill:auto + height:auto means, do we? The spec, prior to #3224 said that column-fill:auto means that we should never balance (except before spanners), didn't it?

@rachelandrew
Copy link
Contributor

Are we in a position to progress on this? I'd like to republish the WD soon as there have been a lot of changes, but one of those is the issue we're considering reverting. So it would be good to make that decision first.

@atanassov atanassov changed the title column-fill should behave more similarly in paginated and continuous contexts [css-multicol] column-fill should behave more similarly in paginated and continuous contexts Aug 27, 2019
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed multicol, and agreed to the following:

  • RESOLVED: revert the change made in #3224, and add spec issues to define this
  • RESOLVED: republish the wd of css-multicol
The full IRC log of that discussion <emilio> Topic: multicol
<emilio> Github: https://github.com//issues/4036
<emilio> rachelandrew: this is an edit made in #3224 (since the last wd)
<emilio> rachelandrew: dbaron opened this, issue has a bunch of discussion
<emilio> rachelandrew: I think it was left reverting the resolution in #3224, but then we need to resolve what column-fill: auto and height: auto works
<emilio> rachelandrew: so we need to decide
<emilio> rachelandrew: I wanted to sort this so I can publish another wd, and I wouldn't like to publish something that we may revert
<emilio> iank_: can you describe what happens right now?
<emilio> rachelandrew: if we're on a continuous context we only look at column-fill: auto in ???, and there were interop issues
<emilio> rachelandrew: Chrome is doing the new behavior, which is actually the old one that was commented out on the spec
<emilio> rachelandrew: dbaron has a nice table on that issue
<emilio> rachelandrew: the second table
<emilio> rachelandrew: I think if we print from the browser the behavior should be the same
<emilio> dbaron: yeah, I think we shouldn't get in that situation, because we'd balance the columns separately because we're printing
<emilio> dbaron: I think at least the last fragment in paginated mode should behave the same as the only fragment
<emilio> dbaron: I suggested a fix, but florian didn't agree because it was not what print formatters do
<emilio> dbaron: so I suggested going in the other direction
<emilio> dbaron: I think mstensho was fine with this if we have a clearer definition of what each combination does
<emilio> dbaron: it'd take me a bit
<emilio> astearns: so reverting would get us to a better place to improve upon right?
<emilio> dbaron: I think so if WebKit are fine with changing this
<emilio> iank_: reverting this leaves less more stuff unspecified right?
<emilio> rachelandrew: yeah, that's right, but we shouldn't publish something that we might revert
<emilio> iank_: can we mark it at risk or such?
<emilio> astearns: it's more of a note "we think this is wrong but we haven't figured out the implications of changing it"
<emilio> iank_: that seems fine for a WD
<emilio> astearns: but if we know it's flat wrong it may not be useful
<emilio> iank_: but is it wrong for sure?
<emilio> dbaron: I think the inconsistency is pretty wrong
<emilio> iank_: mstensho didn't agree looks like
<emilio> dbaron: I don't think he disagreed
<emilio> dbaron: to be clear the thing mstensho is complaining about is that `height: auto; column-fill: auto` if we revert this change
<emilio> dbaron: that combination is already not-defined when you're printing
<emilio> dbaron: so there's an existing spec gap that needs to be filled
<emilio> dbaron: but I don't think it's too hard since there is existing behavior (though we may not like it)
<emilio> florian: I think consistency between printing and formatters is important, so it is consistency between a browser on continuous contexts and printing
<emilio> florian: so we should revert to find a solution that doesn't break any of both
<emilio> astearns: iank_ had objections?
<emilio> iank_: I think reverting is fine
<emilio> astearns: proposed resolution is reverting the change and leave issues in the draft to define this
<emilio> rachelandrew: yeah
<emilio> iank_: may be useful to link to the reverted change
<dbaron> It looks like what Gecko does for the auto height is just to assume one column.
<emilio> RESOLVED: revert the change made in #3224, and add spec issues to define this
<emilio> RESOLVED: republish the wd of css-multicol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-multicol-1 Current Work
Projects
Development

No branches or pull requests

6 participants