Closed
Bug 1063856
Opened 10 years ago
Closed 10 years ago
list-style-type: persian is broken
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla35
People
(Reporter: ebrahim, Assigned: xidorn)
References
()
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(2 files, 2 obsolete files)
110.53 KB,
image/png
|
Details | |
7.35 KB,
patch
|
jfkthame
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Comment 2•10 years ago
|
||
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
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.
Blocks: 966166
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.
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•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.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → quanxunzhen
Status: NEW → ASSIGNED
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
Attachment #8485398 -
Flags: review?(bzbarsky)
Updated•10 years ago
|
Keywords: site-compat
Assignee | ||
Comment 8•10 years ago
|
||
Fix some corrupt counter styles.
Attachment #8485398 -
Attachment is obsolete: true
Attachment #8485398 -
Flags: review?(bzbarsky)
Attachment #8485493 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•10 years ago
|
Version: 33 Branch → Trunk
Updated•10 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 9•10 years ago
|
||
[Tracking Requested - why for this release]: Fix web compatibility broken by landing of bug 966166 in Firefox 33
tracking-firefox33:
--- → ?
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.
Attachment #8485493 -
Flags: review?(bzbarsky) → review?(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.
Attachment #8485493 -
Flags: review?(dbaron)
Updated•10 years ago
|
status-firefox32:
--- → unaffected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox34:
--- → +
tracking-firefox35:
--- → +
Comment 12•10 years ago
|
||
Posted http://lists.w3.org/Archives/Public/www-style/2014Sep/0113.html to see what reactions this gets.
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
Attachment #8485493 -
Attachment is obsolete: true
Attachment #8485493 -
Flags: review?(jfkthame)
Attachment #8485493 -
Flags: review?(dbaron)
Attachment #8486259 -
Flags: review?(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).
Attachment #8486259 -
Flags: review?(jfkthame) → review+
Comment 15•10 years ago
|
||
Ah, you'd done that already; sorry for the noise!
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
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
Attachment #8486259 -
Flags: approval-mozilla-beta?
Attachment #8486259 -
Flags: approval-mozilla-aurora?
Comment 17•10 years ago
|
||
Target Milestone: --- → mozilla35
Updated•10 years ago
|
Keywords: checkin-needed
Comment 18•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 19•10 years ago
|
||
Thank you for fixing this bug, Bug 984005 is worth to see IMO.
Comment 20•10 years ago
|
||
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+
Comment 21•10 years ago
|
||
Updated•10 years ago
|
Flags: qe-verify+
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
Updated•10 years ago
|
Keywords: dev-doc-needed
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
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 24•10 years ago
|
||
I think we need document for the newly added counter styles in this bug.
Keywords: dev-doc-needed
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
Keywords: dev-doc-needed → dev-doc-complete
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•