Scott Johnson (:jwir3) Reporter
	Description • 13 years ago

Gecko's implementation of the CSS3 multicol spec is still prefixed. Once column-span and column-fill have been implemented, we should allow the following CSS properties to be recognized:

column-width
column-count
columns
column-gap
column-rule-color
column-rule-style
column-rule-width
column-rule
column-span
column-fill

	Scott Johnson (:jwir3) Reporter
	Comment 1 • 13 years ago

Manual changes to deprefix columns. Automatic changes to follow.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 2 • 13 years ago

Are the automatic changes going to follow?

Also, what's our testing story here?  The current opinion of the CSS WG is that implementations unprefixing a new feature should be contributing tests for it.  I think we have a bunch of tests that could be relatively-easily massaged into something we could contribute to the WG, though I'm not sure how thorough they are.

	Scott Johnson (:jwir3) Reporter
	Comment 3 • 13 years ago

(In reply to David Baron [:dbaron] from comment #2)
> Are the automatic changes going to follow?

Ah yes, sorry. I will post them right now. I apparently got distracted while I was in the midst of this. My apologies.

> Also, what's our testing story here?  The current opinion of the CSS WG is
> that implementations unprefixing a new feature should be contributing tests
> for it.  I think we have a bunch of tests that could be relatively-easily
> massaged into something we could contribute to the WG, though I'm not sure
> how thorough they are.

Well, I have a number of additional tests (mostly reftests) that I was planning on posting for bug 684062, but I can post these here, if you would prefer. This doesn't seem to be what you're asking about, though. If I understand correctly, when we deprefix we need to provide tests to the CSS WG? There seem to be some tests already available (e.g. http://test.csswg.org/suites/css3-multicol/nightly-unstable/ ). I'm not entirely clear on how the process works here - if we're the first vendor to remove prefixing, is it our responsibility to provide tests? Are these tests then automatically incorporated into the test suite? What is the standard for CSS WG tests (i.e. how much massagin will we have to do to get these to work?)

	Scott Johnson (:jwir3) Reporter
	Comment 4 • 13 years ago

To create this patch, I first moved my .hg directory to one directory higher in the tree, then ran the following command:

find ./ -type f -regextype posix-extended -iregex '.*\.(x?html|css|s?js)' -print | xargs sed -i 's/-moz-column/column/g'

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 7 • 8 years ago

Given changing opinions and policies, is there any reason not to unprefix this right now?

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 8 • 8 years ago

I think that still depends on:
 * whether other browsers are shipping it unprefixed
 * whether their implementations are as buggy as (or more buggy than) ours
 * what sort of bugs we have

	Markus Popp
	Comment 9 • 8 years ago

(In reply to David Baron :dbaron: ⌚️UTC-4 (review requests must explain patch) from comment #8)
> I think that still depends on:
>  * whether other browsers are shipping it unprefixed

Basically every other major browser (Chrome as of version 50.0) is shipping with multicol unprefixed:

http://caniuse.com/#feat=multicolumn

	Jet Villegas (inactive)
	Comment 10 • 8 years ago

(In reply to David Baron :dbaron: ⌚️UTC+1 (mostly busy through August 4; review requests must explain patch) from comment #8)
> I think that still depends on:
>  * whether other browsers are shipping it unprefixed
>  * whether their implementations are as buggy as (or more buggy than) ours
>  * what sort of bugs we have

Neerja will be looking at getting this project over the wire.

	Neerja Pancholi[:neerja]
	Comment 11 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/69464/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69464/

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 12 • 8 years ago

Comment on attachment 8778065 [details]
Bug 698783 - Updating the existing tests from CSS Working Group (Step 2a from dbarons comment#12)

https://reviewboard.mozilla.org/r/69464/#review66906

So as we discussed earlier today, I think there are a few things to do here that should be separated into a few different patches:

 (1) we should get rid of the _moz_ in the internal constants for these properties, anytime.  That's just code cleanup.

 (2) we should import the multicol reftests from the CSSWG test repository.  This has a few steps:
     (a) rerun layout/reftests/w3c-css/import-tests.py , and if there are any changes, make a patch with them and push it to try.
     (b) then, add importing of the multicol tests, and do the same.  This depends on (2a).
     (c) evaluate the failures of the multicol tests added in (b) and figure out if we're doing worse than other browsers that have unprefixed.  This depends on (2b).

 (3) write a patch to layout/style/nsCSSPropList.h to remove the -moz- prefixes there, but add them back as aliases in layout/style/nsCSSPropAliasList.h, and then make the same changes to layout/style/test/property_database.js and any other tests that show failures (probably a small number).  This depends on (2c) to decide whether we're ready to do it.

 (4) Substitute all of the rest of the tree to remove the -moz- prefixes for these properties.  This depends on (3).

 (5) Remove the aliases introduced in (3).  This depends on (3) and (4), and shouldn't be done until after we've successfully shipped (3).

All of these (1, 2a, 2b, 2c, 3, 4, and 5) could be different bugs, although it's ok if 2a and 2b are in a single bug, and it's also ok if 3 and 4 are in a single bug, or even 1, 3, and 4.  But things that happen at different times should be in separate bugs even if they could be in the same bug if done at the same time.

	Daniel Holbert [:dholbert]
	Comment 13 • 8 years ago

Comment on attachment 8778065 [details]
Bug 698783 - Updating the existing tests from CSS Working Group (Step 2a from dbarons comment#12)

(per our IRC discussion yesterday, I think you meant to cancel the review-request here.  --> canceling r?dholbert flag.)

	Kohei Yoshino [:kohei]
	Comment 16 • 8 years ago

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/prefixed-css3-multi-column-properties-will-be-removed/

	Gérard Talbot
	Comment 18 • 7 years ago

(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #12)

>      (c) evaluate the failures of the multicol tests added in (b) and figure
> out if we're doing worse than other browsers that have unprefixed. 


Current support of CSS3 Multi-column test suite (170 tests)
http://test.csswg.org/harness/review/css-multicol-1_dev/
with Firefox 57.0a1 buildID=20170820221144:

http://test.csswg.org/harness/results/css-multicol-1_dev/grouped/browser/Firefox/browser_version/57.0/

Gecko: Passed 66.47%, Coverage 100%

- - - - - - - - -

Other results:
http://test.csswg.org/harness/results/css-multicol-1_dev/grouped/

- - - - - - - - -

Current support of CSS3 Multi-column test suite (170 tests)
http://test.csswg.org/harness/review/css-multicol-1_dev/
with Chrome 62.0.3188.2 (dev channel):

http://test.csswg.org/harness/results/css-multicol-1_dev/grouped/engine/Blink/browser/Chrome/browser_version/62.0.3188.2/

Blink: Passed 70.59%, Coverage 100%



Preliminary comments on a few of those 170 tests
************************************************

http://test.csswg.org/harness/test/css-multicol-1_dev/multicol-width-ems-001/
is most likely (very high probability) that it is an imprecise test or the reference file is imprecise.

- - - - - - - - -

http://test.csswg.org/harness/test/css-multicol-1_dev/multicol-rule-fraction-003/
is imprecise. The reference file

http://test.csswg.org/suites/css-multicol-1_dev/nightly-unstable/html4/reference/multicol-rule-fraction-3-ref.htm

has its leftmost vertical blue stripe most likely is off by 1px because 
of fractional pixel:

line 34: #a1 {left: 2.43em;}
is off by 1px. Looks okay with left: 2.4em instead.
The whole reftest should probably be recoded to avoid fractional pixel. The reftest has _nothing to do_ with column-rule and has everything to do with absolute positioning.

- - - - - - - - -

wrong linkage of reference file in

http://test.csswg.org/harness/test/css-multicol-1_dev/multicol-fill-auto/

- - - - - - - - -

http://test.csswg.org/harness/test/css-multicol-1_dev/multicol-count-large-001/

http://test.csswg.org/harness/test/css-multicol-1_dev/multicol-count-large-002/

fail by 1px only in Firefox 57 and Chrome 62. Such 1px failure is most likely due to the intrinsic nature of the tests themselves. I do not think failing these 2 tests should mean anything, except that the tests are extreme, testing extreme limits.