Closed Bug 715203 Opened 8 years ago Closed 8 years ago

-moz-column-fill property does not update from within script correctly


(Core :: Layout, defect, P3)






(Reporter: jwir3, Assigned: jwir3)



(Whiteboard: [css3-multicol])


(1 file)

Viewing tbpl errors (example given by philor was a ts failure on android), the results section in the lower right hand corner is now set to 'balance', when it was previously 'auto', prior to the landing of bug 695222. This is expected, since the spec dictates that we should default to 'balance'. However, when attempting to do:

document.getElementById('results').style.MozColumnFill = "auto"

No visible change happens to the results div element. Similarly, a follow-up:

window.getComputedStyle(document.getElementById('results')).getPropertyValue('-moz-column-fill') shows 'auto', as expected, but, again, no change is visible.
Hmm.  We do have a reflow hint for column-fill.  Is that reflow happening?
I don't think it's just "from script" - my dev copy of tbpl has -moz-column-fill: auto; in the stylesheet, and it still gets balanced.
(In reply to Phil Ringnalda (:philor) from comment #2)
> I don't think it's just "from script" - my dev copy of tbpl has
> -moz-column-fill: auto; in the stylesheet, and it still gets balanced.

That's really strange, because there are unit tests that check this. Specifically, there are reftests that check that the behavior of column-fill is different on auto than balance (layout/reftests/columns/columnfill-auto and columnfill-balance), as well as checking the behavior is correct for both auto and balance. Additionally, there's a reftest that checks to see if changing the column-fill property has an effect on the layout (layout/reftests/columns/columnfill-change). AFAICT, these aren't failing...
Blocks: 715434
Sure, but do you have reftests for ${whatever unexpected thing makes it not work on tbpl}? To pick one random possibility, says there's not a single ul in the columns reftests, and I'll bet there aren't any that fill the column by setting the .innerHTML, which I think is what tbpl does.
Maybe minimized version of tbpl (maybe I have no idea what I'm doing, too): 

data:text/html,<div style="-moz-column-width: 40em; overflow-x: auto; -moz-column-fill: auto;"><li>one<li>two<li>three</div>

Without the overflow-x: auto, that's one column; with, it's three.
Ah, yes.  For scrollable columnated stuff, the column styles are on the scrollframe, not on the columnset.

If you look at the ua.css rules for -moz-scrolled-*, they explicitly inherit column-count, column-width, column-gap, and column-rule.  Presumably they need to also inherit column-fill.
Yep, that plus bug 715434 does indeed fix tbpl.
Ok, so I fixed this in a patch, but I'd also like to write a quick reftest. How do I invoke this -moz-scrolled-content property? I can't seem to find any useful documentation on it...
Use any value of "overflow" other than "visible", pretty much.  See comment 5, for example.
Attachment #586867 - Flags: review?(bzbarsky)
Comment on attachment 586867 [details] [diff] [review]
b715203 - Patch and test.

Probably better to call the test file something like column-fill-with-overflow-style.html or something.

And does this need to be an HTTP test?  If it really does, maybe document why?

r=me with that.
Attachment #586867 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #11)

> And does this need to be an HTTP test?  If it really does, maybe document
> why?
Yes, actually. It uses the ahem font, which is located in layout/reftests/fonts. I'll document it.
Hmm.  It doesn't have @font-face rules for that font, so why is HTTP involved?  (I might just be misunderstanding part of the test harness setup, of course.)
Ah, yes, you are correct. I neglected to add that rule, and it was still passing. I've removed the font-family from the html files, and HTTP from reftest.list. :)
Pushed to inbound:
Target Milestone: --- → mozilla12
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.