ebrahim Reporter
	Description • 10 years ago

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20140905030206

Steps to reproduce:

Open data:text/html,<ol style="list-style-type: -moz-persian;"><li></li><li></li></ol><ol style="list-style-type: -moz-persian; list-style-type: persian;"><li></li><li></li></ol>


Actual results:

On Firefox 33 and above (mine is nightly 35), first is with Persian digits and second not


Expected results:

As Firefox 32, both should look same. Seems non prefixed `persian` value is supported recently but is actually broken.

	Gordon P. Hemsley [:GPHemsley]
	Comment 2 • 10 years ago

I get the same results in Aurora 33 on Linux.

	Boris Zbarsky [:bzbarsky]
	Comment 3 • 10 years ago

> Seems non prefixed `persian` value is supported

It's not.  You get the same result with "list-style-type: -moz-persian; list-style-type: pokemon", say.

What happened is that bug 966166 got fixed, which allows pages to define custom counter styles and use them as list-style-type values.  So now any identifier will be parsed; if it's a known built-in style that will be used, otherwise it will be treated as a custom counter style.  If the page doesn't actually define such a counter style, it falls back to the decimal style.

As far as I can tell, this is the behavior the spec requires.  See http://dev.w3.org/csswg/css-counter-styles/#extending-css2 which says:

  If a <counter-style-name> is used that does not refer to any existing counter style,
  it must act identically to the decimal counter style.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 4 • 10 years ago

Boris is right. The solution is to insert the defination of persian from http://www.w3.org/TR/predefined-counter-styles/#persian and leave -moz-persian for compatibility.

Or since -moz-persian has been defined in Firefox, you can simply use

@counter-style persian { system: extends -moz-persian; }

but it will not be compatible with other browsers in the future.

	ebrahim Reporter
	Comment 5 • 10 years ago

It is used on Wikipedia and every MediaWiki https://github.com/wikimedia/mediawiki-core/blob/a0c6a63e7f4231c864e56af6da5289467593065b/resources/src/mediawiki.legacy/shared.css#L942 What do you suggest for it, first stating non-prefixed and then prefixed? Wasn't better if Firefox doesn't recognize `persian`, would ignore it and use prefixed style?

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 6 • 10 years ago

I think the current behavior is basically fine, while we can do something to prevent the breakage.

I suggest that we can add the unprefixed version of those styles, as they have been defined in Predefined Counter Styles, and been supported unprefixed in Chrome and Safari.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 7 • 10 years ago

This patch adds all counter styles which are defined in Predefined Counter Styles list, and have been included unprefixed in WebKit and Blink.

Counter styles support in browsers: http://www.w3.org/International/tests/repository/predefined-counter-styles/results-predefined-counter-styles

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 8 • 10 years ago

Fix some corrupt counter styles.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 9 • 10 years ago

[Tracking Requested - why for this release]: Fix web compatibility broken by landing of bug 966166 in Firefox 33

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 10 years ago

Comment on attachment 8485493 [details] [diff] [review]
patch

Going to defer to the original reviewers of the counter styles bits.

	Jonathan Kew [:jfkthame]
	Comment 11 • 10 years ago

Comment on attachment 8485493 [details] [diff] [review]
patch

Review of attachment 8485493 [details] [diff] [review]:
-----------------------------------------------------------------

This (and the corresponding blink/webkit behavior) conflicts with the current CSS Counter Styles draft[1], which says, in reference to the i18n WG's Predefined Counter Styles document, "These additional counter styles are _not_ intended to be supported by user-agents by default, but can be used by users or authors copying them directly into style sheets." [my emphasis]

If we make them built-ins, we'll be encouraging authors to use them without providing the necessary definitions in their own style sheets, and thus breaking compatibility with the spec as it currently stands.

Maybe the fact that blink/webkit already do this is enough to make it a de facto "standard", and we should just follow them? But if so, then CSS Counter Styles should be updated to include these as standard styles.

If we are going to do this, I have a few minor comments (below) on the actual patch; but first I think we need to consider the spec implications. Flagging :dbaron for any thoughts on the way forward here.

[1] http://dev.w3.org/csswg/css-counter-styles/#additional-predefined

::: layout/style/counterstyles.css
@@ +186,5 @@
> +
> +@counter-style cambodian {
> +  system: numeric;
> +  symbols: \17E0  \17E1  \17E2  \17E3  \17E4  \17E5  \17E6  \17E7  \17E8  \17E9;
> +}

AFAICS, 'cambodian' is identical to 'khmer', so it would be marginally more compact to define it using 'system: extends khmer'.

@@ +226,5 @@
> +
> +@counter-style oriya {
> +  system: numeric;
> +  symbols: \B66  \B67  \B68  \B69  \B6A  \B6B  \B6C  \B6D  \B6E  \B6F;
> +}

This collection seems to be mostly alphabetized, but 'oriya' is misplaced; please move it up before 'telugu'.

@@ +242,4 @@
>  }
>  
>  @counter-style -moz-urdu {
>    system: extends -moz-persian;

Make this 'extends persian', to avoid the extra indirection.

@@ +267,5 @@
>  
>  @counter-style -moz-tamil {
>    system: numeric;
>    symbols: \BE6  \BE7  \BE8  \BE9  \BEA  \BEB  \BEC  \BED  \BEE  \BEF;
>  }

I think we should support 'tamil' in the unprefixed list, just like the other major Indian languages (or script names); there's no good reason (from a user's/author's point of view) why it should be an odd one out. Then -moz-tamil will simply be an alias, like the other Indic script names. If webkit/blink is lagging behind on this one (as indicated by the test-results page), so be it.

	Jonathan Kew [:jfkthame]
	Comment 12 • 10 years ago

Posted http://lists.w3.org/Archives/Public/www-style/2014Sep/0113.html to see what reactions this gets.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 13 • 10 years ago

As Tab said so [1], I think it is fine to go forward.

[1] http://lists.w3.org/Archives/Public/www-style/2014Sep/0123.html

	Jonathan Kew [:jfkthame]
	Comment 14 • 10 years ago

Comment on attachment 8486259 [details] [diff] [review]
patch

Review of attachment 8486259 [details] [diff] [review]:
-----------------------------------------------------------------

OK, let's do it. See comment 11; r=me with those (in particular, include 'tamil' along with the other Indic styles, for consistency).

	Jonathan Kew [:jfkthame]
	Comment 15 • 10 years ago

Ah, you'd done that already; sorry for the noise!

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 16 • 10 years ago

Comment on attachment 8486259 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 966166
[User impact if declined]: There will be some site compatibility problems due to landing of @counter-style.
[Describe test coverage new/current, TBPL]: AFAIK, this change is not covered by any test in our repo currently, but it can pass the related tests provided by W3C International group.
[Risks and why]: AFAICS, there is no risk, since the changed file first appears in Firefox 33.
[String/UUID change made/needed]: N/A

	Jonathan Kew [:jfkthame]
	Comment 17 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/8922b7cbc32d

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/8922b7cbc32d

	ebrahim Reporter
	Comment 19 • 10 years ago

Thank you for fixing this bug, Bug 984005 is worth to see IMO.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 20 • 10 years ago

Comment on attachment 8486259 [details] [diff] [review]
patch

Approved for beta and aurora.

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/d81a54adae66
https://hg.mozilla.org/releases/mozilla-beta/rev/8e9b139e30b9

	Cristina Madaras, QA [:CristinaM]
	Comment 22 • 10 years ago

Reproduced the issue on Windows 7 64bit.
Builds:
Nightly
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:35.0) Gecko/20100101 Firefox/35.0
Build Id: 20140905030206

Aurora
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
Build Id: 20140905004002

Beta
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Build Id: 20140908190852

Verified as fixed on
Latest Nightly:
Build Id: 20140911064110

Windows 7 64bit User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:35.0) Gecko/20100101 Firefox/35.0
Ubuntu 14.04 32bit User Agent: Mozilla/5.0 (X11; Linux i686; rv:35.0) Gecko/20100101 Firefox/35.0
Mac OS 10.8.5 User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:35.0) Gecko/20100101 Firefox/35.0

Beta 33.0b3:
BuildId: 20140911191954

Windows 7 64bit User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Ubuntu 14.04 32bit User Agent: Mozilla/5.0 (X11; Linux i686; rv:33.0) Gecko/20100101 Firefox/33.0
Mac OS 10.8.5 User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:33.0) Gecko/20100101 Firefox/33.0

I will verify this on Aurora when the build will be available

	Cristina Madaras, QA [:CristinaM]
	Comment 23 • 10 years ago

Verified as fixed on Aurora Build Id: 20140914004004 under:

Windows 7 x64bit User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0
Ubuntu 14.04 x32bit User Agent: Mozilla/5.0 (X11; Linux i686; rv:34.0) Gecko/20100101 Firefox/34.0
Mac OS 10.8.5 User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:34.0) Gecko/20100101 Firefox/34.0

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 24 • 10 years ago

I think we need document for the newly added counter styles in this bug.

	Jean-Yves Perrier [:teoli]
	Comment 25 • 10 years ago

I have updated the page with the following unprefixing information:
persian, arabic-indic, devanagari, bengali, gurmukhi, gujarati, oriya, tamil, telugu, kannada, malayalam, thai, lao, myanmar, khmer, cjk-heavenly-stem, cjk-earthly-branch 

and

with the new mongolian value.

I hope I've read the bug correctly :-).

So updated:
https://developer.mozilla.org/en-US/docs/Web/CSS/list-style-type
and
https://developer.mozilla.org/en-US/Firefox/Releases/33

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 26 • 10 years ago

(In reply to Jean-Yves Perrier [:teoli] from comment #25)
> I have updated the page with the following unprefixing information:
> persian, arabic-indic, devanagari, bengali, gurmukhi, gujarati, oriya,
> tamil, telugu, kannada, malayalam, thai, lao, myanmar, khmer,
> cjk-heavenly-stem, cjk-earthly-branch 
> 
> and
> 
> with the new mongolian value.
> 
> I hope I've read the bug correctly :-).
> 
> So updated:
> https://developer.mozilla.org/en-US/docs/Web/CSS/list-style-type
> and
> https://developer.mozilla.org/en-US/Firefox/Releases/33

I haven't detailedly checked the new document, but it overall looks good to me. But I think the compatibility table is not so correct. All of the styles, except tamil, added in this bug has been supported in Chrome and Safari for a long time. In addition, some of the new styles do not have prefixed version before.