822053 - List breaks within nested columns

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[cwebba1]

Attachment: bugzilla.htm

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

List breaks within nested columns
I have a 2-column div within a box div that is nested within a 2-column div. The nested column contains a ul list. The list is breaking within the nested column.

I've pasted my 2-column box outside of the containing 2-column div. It works outside of the first 2-column containing div.


Actual results:

I have created an example here: http://www.craigwebbart.com/ga/bugzilla.htm

The working box is to the left. The broken nested 2-column box with list is to the right.


Expected results:

The example on the left is what I'm aiming for. I need the 2-column elements to revert to 1-column using media queries. This works fine in Safari 5.0.6.

Please let me know if this bug exists in newer versions of Firefox. Also, please alert me to a work-around or patch-fix. Thanks!

Comment 1

[Mats Palmgren (inactive)]

This broke some time between Fx 12 and 13, maybe bug 725376?
But we should allow two nested balancing column sets... and for the URL,
there is a specified -moz-column-count:2 on both so I would expect that
to NOT be affected by the code bug 725376 added.

Comment 2

[Loic]

(In reply to Mats Palmgren [:mats] from comment #1)
> This broke some time between Fx 12 and 13, maybe bug 725376?

Confirmed:

m-c
good=2012-02-18
bad=2012-02-19
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=550779e6bab4&tochange=4d47329bb02e

Suspected bug:
Mats Palmgren — Bug 725376 - Don't do column balancing deeper than 2 nested levels to avoid hang. r=roc

Comment 3

[cwebba1]

Thank you men for looking into this. Here's the full list of math needs that I used on the page:
All elements: box-sizing: border-box
Widths set a percentage (responsive)
2-column box nested within 2 column box
UL list with special character inserted

The special character in the first list item is disappearing in webkit. Other than that the list splits well into two columns.
If I defined a height in the inner box would that help?

Comment 4

[Mats Palmgren (inactive)]

> If I defined a height in the inner box would that help?

Unfortunately not.  When bug 733614 backed out bug 695222 the "is balancing"
test was lost.  It should have just changed the assignment of 'isBalancing'
to 'aReflowState.ComputedHeight() == NS_INTRINSICSIZE'.
http://hg.mozilla.org/mozilla-central/diff/aeaa00b5cab5/layout/generic/nsColumnSetFrame.cpp
Then I think the workaround you suggest would have worked.

Comment 5

[Mats Palmgren (inactive)]

Attachment: fix+test

The counting of levels was wrong, and the isBalancing test needs to be
restored.  Now with regression tests!
https://tbpl.mozilla.org/?tree=Try&rev=cbc245143c38

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix for beta

Same fix for beta, but with isBalancing like so:
  isBalancing = aReflowState.ComputedHeight() == NS_INTRINSICSIZE;

In case we want it on beta...

Comment 7

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d0cc82f292e

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 695680 [details] [diff] [review]
fix+test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 725376
User impact if declined: bad layout in nested column sets, no workaround
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): very low risk
String or UUID changes made by this patch: none

I think the risk/benefit makes it worth taking on Aurora at least.

Comment 9

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/6d0cc82f292e

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 695680 [details] [diff] [review]
fix+test

Approving on aurora considering comment #8.

Comment 11

[bhavana bajaj [:bajaj]]

(In reply to cwebba1 from comment #3)
> Thank you men for looking into this. Here's the full list of math needs that
> I used on the page:
> All elements: box-sizing: border-box
> Widths set a percentage (responsive)
> 2-column box nested within 2 column box
> UL list with special character inserted
> 
> The special character in the first list item is disappearing in webkit.
> Other than that the list splits well into two columns.
> If I defined a height in the inner box would that help?

looks like we have a fix now, can you please help with some testing here to verify it works as expected & nothing new was regressed ? Thank you in advance !

Comment 12

[Mats Palmgren (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8c2c113e0d53