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

RESOLVED FIXED in mozilla12

Status

()

Core
Layout
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jwir3, Assigned: jwir3)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [css3-multicol])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 3

5 years ago
(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, http://mxr.mozilla.org/mozilla-central/search?find=%2Flayout%2Freftests%2Fcolumns%2F&string=%3Cul 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.
(Assignee)

Comment 8

5 years ago
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.
(Assignee)

Comment 10

5 years ago
Created attachment 586867 [details] [diff] [review]
b715203 - Patch and test.
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+
(Assignee)

Comment 12

5 years ago
(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.)
(Assignee)

Comment 14

5 years ago
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. :)
(Assignee)

Comment 15

5 years ago
Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3244d19894c
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/d3244d19894c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.