list-style-type: persian is broken

VERIFIED FIXED in Firefox 33

Status

()

Core
CSS Parsing and Computation
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ebrahim, Assigned: xidorn)

Tracking

({dev-doc-complete, site-compat})

Trunk
mozilla35
dev-doc-complete, site-compat
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox32 unaffected, firefox33+ verified, firefox34+ verified, firefox35+ verified)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 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.
(Reporter)

Comment 1

4 years ago
Created attachment 8485316 [details]
32 vs 35
I get the same results in Aurora 33 on Linux.
Status: UNCONFIRMED → NEW
Component: General → CSS Parsing and Computation
Ever confirmed: true
OS: Windows 8.1 → All
Hardware: x86_64 → All
> 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.
Blocks: 966166
(Assignee)

Comment 4

4 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.
(Reporter)

Comment 5

4 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?
(Assignee)

Comment 6

4 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.
(Assignee)

Updated

4 years ago
Assignee: nobody → quanxunzhen
Status: NEW → ASSIGNED
(Assignee)

Comment 7

4 years ago
Created attachment 8485398 [details] [diff] [review]
patch

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
Attachment #8485398 - Flags: review?(bzbarsky)
Keywords: site-compat
(Assignee)

Comment 8

4 years ago
Created attachment 8485493 [details] [diff] [review]
patch

Fix some corrupt counter styles.
Attachment #8485398 - Attachment is obsolete: true
Attachment #8485398 - Flags: review?(bzbarsky)
Attachment #8485493 - Flags: review?(bzbarsky)
(Assignee)

Updated

4 years ago
Version: 33 Branch → Trunk

Updated

4 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 9

4 years ago
[Tracking Requested - why for this release]: Fix web compatibility broken by landing of bug 966166 in Firefox 33
tracking-firefox33: --- → ?
Comment on attachment 8485493 [details] [diff] [review]
patch

Going to defer to the original reviewers of the counter styles bits.
Attachment #8485493 - Flags: review?(bzbarsky) → review?(jfkthame)
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.
Attachment #8485493 - Flags: review?(dbaron)
status-firefox32: --- → unaffected
status-firefox33: --- → affected
status-firefox34: --- → affected
status-firefox35: --- → affected
tracking-firefox33: ? → +
tracking-firefox34: --- → +
tracking-firefox35: --- → +
(Assignee)

Comment 13

4 years ago
Created attachment 8486259 [details] [diff] [review]
patch

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
Attachment #8485493 - Attachment is obsolete: true
Attachment #8485493 - Flags: review?(jfkthame)
Attachment #8485493 - Flags: review?(dbaron)
Attachment #8486259 - Flags: review?(jfkthame)
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).
Attachment #8486259 - Flags: review?(jfkthame) → review+
Ah, you'd done that already; sorry for the noise!
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 16

4 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
Attachment #8486259 - Flags: approval-mozilla-beta?
Attachment #8486259 - Flags: approval-mozilla-aurora?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8922b7cbc32d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox35: affected → fixed
Resolution: --- → FIXED
(Reporter)

Comment 19

4 years ago
Thank you for fixing this bug, Bug 984005 is worth to see IMO.
Comment on attachment 8486259 [details] [diff] [review]
patch

Approved for beta and aurora.
Attachment #8486259 - Flags: approval-mozilla-beta?
Attachment #8486259 - Flags: approval-mozilla-beta+
Attachment #8486259 - Flags: approval-mozilla-aurora?
Attachment #8486259 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
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
status-firefox33: fixed → verified
status-firefox35: fixed → verified

Updated

4 years ago
Keywords: dev-doc-needed
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
Status: RESOLVED → VERIFIED
status-firefox34: fixed → verified
(Assignee)

Comment 24

4 years ago
I think we need document for the newly added counter styles in this bug.
Keywords: dev-doc-needed
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
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 26

4 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.
You need to log in before you can comment on or make changes to this bug.