298893 - -moz-column strips initial / final vertical spacing.

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Jason Bassford]

When a -moz-column div is inside another div, it strips out initial and leading
vertical whitespace.  Per a comment at MozillaZine, "Basically, using columns
seems to break the rules for collapsing margins."

Testcase to follow.

Assigning this to Roc who worked on bug 251162.

Comment 1

[Jason Bassford]

Attachment: Initial and final vertical whitespace / margins lost.

(Summary should have said "initial and final" rather than "initial and
leading".)

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There's definitely a bug where the top margin of the leading P and the bottom
margin of the trailing P are being discarded. That's actually easy to fix. If we
want margins to collapse across column-set boundaries then I don't really know
how that should work.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: screenshot of testcase, after applying fix

As you can see, just making the margins appear inside the columns isn't
terribly pleasing to the eye.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think the best results would actually be to ignore the leading and trailing
margins as we currently do, and have the author apply padding to the columns
element.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In fact, what we currently do is ignore the leading and trailing margins on each
individual column, which seems like a really good idea.

I *suppose* you could imagine doing something like "take the max of the leading
margins as the top margin for the entire column set, take the max of the
trailing margins as the bottom margin for the entire column set" but that's
really hard to implement because it means you don't know the top margin before
you start laying out the columns, which means you don't know where to place the
columns before you lay them out, which is really really bad.

A more reasonable alternative would be to let the leading margin of the first
column collapse out to become the top margin of the column set, and let the
trailing margin of the last column collapse out to become the bottom margin of
the column set, and ignore the leading and trailing margins of the other
columns. That shouldn't be too hard to implement.

I'll listen to any proposal, but I think what we're doing is no worse than any
of the alternatives I've thought of so far.

Comment 6

[fantasai]

Not having thought about it much, your last proposal seems to strike a pretty
good balance, and will also, I think, give authors the least surprising results
when their pages get laid out.

Comment 7

[Jason Bassford]

> As you can see, just making the margins appear inside the columns isn't
> terribly pleasing to the eye.

Yes, that's not good (worse than current behaviour I think)...

> I think the best results would actually be to ignore the leading and trailing
> margins as we currently do, and have the author apply padding to the columns
> element.

My problem, personally, is when I try to design my page to be compatible with
both Mozilla and IE (or other browsers, or older versions of Mozilla) where
-moz-columns isn't supported.  If I only cared about IE then I could deal with
things as per the above, but for cross-browser compatibility I end up with a
situation where I force things to be "as before" by adding padding to the
-moz-column div - but that ends up giving me *extra* space in the other
rendering engines since no spacing is removed in the first place by them (they
ignore the -moz-column which does this).

Still, I can live with this, even if it's not quite "right".

Is there any chance that we could add a -moz-column-padding directive, or
something like that, where we could add back in the vertical space that's taken
away?  (I know that it's not really padding-top and padding-bottom that's (not)
taking place - but, in terms of layout, it has the same effect.)  That way, if a
browser doesn't understand the directive, nothing happens at all; if it does,
then the top/margins disappear - but can then be specifically put back in again.

Comment 8

[Jason Bassford]

Actually, I've given this some more thought.

If somebody is going to have an initial <p> inside of a -moz-column frame, then,
yes, your patched screenshot *IS* what I would expect to happen.  If somebody
doesn't like that, then they should just not have that initial <p>.

I can easily see somebody complaining because they created columns of text and
*do* want it to look like your screenshot - where only the first column has
vertical whitespace due to the leading <p>.  After all, that's what happens
without columns and, appearances aside, what I'd expect to happen from an
absolutely technical point of view.

The fact that it's not what *I* want it to turn into is sort of beside the
point.  I simply need to work around that and come up with another method for my
personal situation.  Such as having a -moz-column-header / -moz-column-footer
(or moz-column-padding-top), etc., specific addition as I'd suggested earlier. 
But that could well be an entirely different bug that should be filed.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm going to try implementing my second option. It has the nice property that it
makes a single column behave just like a regular block. It will also "do the
right thing" for people like Jason who want things to degrade nicely for
browsers without columns.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes things so that the first column's top margin collapses with
predecessors and the last column's bottom margin collapses with successors.
This produces the effect we want. It should be possible to stare at the patch
and see that it doesn't change anything in the absence of columns.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 189393 [details] [diff] [review]
fix

r+sr=dbaron, but you might consider using the pres shell's stack arena for the
reflow state allocation.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 189393 [details] [diff] [review]
fix

a fix that makes columns more usable and degrade more gracefully on browsers
that don't support columns. Should only affect columns

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is a rarely used code path so I don't think we need to use the stack allocator.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 15

[Jason Bassford]

Thanks!