(In reply to Jean-Yves Perrier [:teoli] from comment #5)
> Neerja, to evaluate the doc needs for this bug, does this bug actually
> unprefix the properties (keeping the prefixed versions as aliases)?
Yes, that is correct.
This is a step in the direction towards removing all the multicol prefixes altogether (Bug 698783) and as part of this step we will still have aliases to prefixed multicol properties. Eventually, these too will be removed.

Drive-by nit on the commit message:
> Bug 1300895 - Remove -moz- prefix from nsCSSPropList.h and any other
> failing tests, add corresponding aliases to nsCSSPropAliasList.h
This should include the phrase "multi-column properties" somewhere, to make it clearer what it's doing.
Perhaps update to something like:
"Unprefix CSS multi-column properties, but add back prefixed aliases via nsCSSPropAliasList.h"
(The test tweaks are nice to mention, but they might make the message a bit too long, particularly after we've added the clarifying bit about multi-column properties. So, probably ok to just leave those out of the message.)

(In reply to Daniel Holbert [:dholbert] from comment #7)
> Drive-by nit on the commit message:
> > Bug 1300895 - Remove -moz- prefix from nsCSSPropList.h and any other
> > failing tests, add corresponding aliases to nsCSSPropAliasList.h
>
> This should include the phrase "multi-column properties" somewhere, to make
> it clearer what it's doing.
>
> Perhaps update to something like:
> "Unprefix CSS multi-column properties, but add back prefixed aliases via
> nsCSSPropAliasList.h"
>
> (The test tweaks are nice to mention, but they might make the message a bit
> too long, particularly after we've added the clarifying bit about
> multi-column properties. So, probably ok to just leave those out of the
> message.)
Ok, I changed the commit message to what you suggested. It should reflect the next time I push for review. I will add a summary of the test related changes in the lines following the commit message (so that the commit message itself isn't too long) Does that work?

Comment on attachment 8799055[details][diff][review]Bug 1300895: followup to update devtools' property database
Aryx initially landed this for us, but I'm very uneasy about it because this devtools file is impossible to usefully diff / audit, which makes me uncomfortable taking changes to it in a bustage fix. e.g. this file has lines that are over 100,000 characters long, which makes it impossible to know whether (autogenerated) changes to it make any sense at all.
Let's fix this file in bug 1308651 so that it can be usefully audited, and then update the main patch here to update this test file alongside the other test files, and then re-land. (Hopefully on Monday or early next week, if we can get the file format fixed quickly.)

I think for now, we're gated on a fix to bug 1308651 (which hopefully can come soon -- if gtatum can't get to it soon, perhaps neerja can reverse-engineer how the file is generated, and tweak the script to include some conveniently-placed newlines characters, so that changes can be represented in more targeted patches/hg-changesets)
--> adding dependency

Hi Daniel,
This is a try run for the "Followup to update devtools' property database" patch before the fix for properties-db.js formatting (Bug 1308651) was applied. I did not re-run try after this formatting fix + rebase on top of autoland patches since I think the result will be equivalent. Let me know if you need me to rerun this.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd2c197bbe70
Thanks!

This looks good! Two notes:
(1) please fold the followup into the main patch, as we just discussed. (The changes vs. the last reviewed version are small/obvious enough that I'm sure dbaron is OK with you carrying forward r=dbaron, particularly since I've sanity-checked them as well.)
(2) the followup does include an unrelated chunk for "layout.css.masking.enabled". That seems to be the script "catching up" this generated file to changes that already happened in bug 1308239. Ideally we might want to split that change out (e.g. run the script before the rest of your changes here [which would probably just produce this one masking tweak], commit that, then apply the rest of your changes and run the script again). But it doesn't matter too much, so let's not worry about it.

(In reply to Daniel Holbert [:dholbert] from comment #25)
> (2) the followup does include an unrelated chunk for
> "layout.css.masking.enabled" [...] let's not worry about it.
(Following up on this -- that about:config pref metadata is probably all getting removed from this devtools file soon anyway, per bug 1309040, and that means we shouldn't have many more instances of "catch-up" tweaks like this getting rolled into other unrelated patches. Which is great.)

Yeah, it looks like that test is operating on the top chunk of the properties-db.js file (the exports.CSS_PROPERTIES definition), which the script didn't update for some reason.
In the latest version of the patch here, the (script-generated) unprefixing changes to properties-db.js are all in exports.PREFERENCES, which don't matter for this test. So, we need to figure out why the script isn't updating exports.CSS_PROPERTIES, and fix it to do that (perhaps in a helper-bug).

(I filed bug 1309065 on addressing part of comment 30. In the meantime, it seems like we can make "./mach devtools-css-db" produce a more useful/complete update [for the purposes of this bug], if we clear the data from its structures before running it.)

Hi Greg,
I generated a new properties-db.js file using the workaround that dholbert mentioned in Bug 1309065 comment#1 (at the end) and uploaded it here. Can you please sanity check this file for us to make sure that this might be a feasible work around? I ran this xpcshell test locally and I do see that it passes after this patch.
There is also a try run for this at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca938d82b741
Thanks!
Neerja

(In reply to info from comment #44)
> There is a bug on Firefox 52 with CSS columns. If the container has
> column-fill: auto it breaks the columns. It was not breaking the columns in
> version 51.0.1.
-moz-column-fill: balance
column-fill: auto

(In reply to info from comment #45)
> -moz-column-fill: balance
> column-fill: auto
If that's what you specified, then you'll get different behavior in 51 and 52, because 51 will get 'balance' and 52 (which supports unprefixed 'column-fill') will get 'auto'.

Put another way: if you've got comment 45's CSS and you want the Firefox 51 behavior, then you should just have:
-moz-column-fill: balance
column-fill: balance
(Alternately/also, if you do really want "auto" and it does the right thing in other browser engines but not in Firefox 52, then please file a bug about that and mention the bug number here:
https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=Layout )