Support string value on list-style-type

RESOLVED FIXED in Firefox 39

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

({dev-doc-complete})

Trunk
mozilla39
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed, relnote-firefox 39+)

Details

(URL)

Attachments

(3 attachments)

(Assignee)

Description

4 years ago
CSS List 3 extends the syntax of list-style-type (as well as its shorthand property list-style) to accept a string value. It should be quite easy for use to implement since we have implemented CSS Counter Styles, and the string value is just a special case in cyclic system.
(Assignee)

Comment 1

4 years ago
Created attachment 8579717 [details] [diff] [review]
patch 1 - remove BuildCounterStyle for anonymous counter styles
Assignee: nobody → quanxunzhen
Attachment #8579717 - Flags: review?(dbaron)
(Assignee)

Comment 2

4 years ago
Created attachment 8579720 [details] [diff] [review]
patch 2 - handle string value on list-style-type
Attachment #8579720 - Flags: review?(dbaron)
(Assignee)

Comment 3

4 years ago
Created attachment 8579721 [details] [diff] [review]
patch 3 - reftests
Attachment #8579721 - Flags: review?(dbaron)
(Assignee)

Comment 4

4 years ago
According to Simon Sapin [1] and Tab Atkins Jr. [2], this part of the spec should be stable enough to be implemented. And given we have implemented the Counter Styles module, it doesn't cost much code for us to add support for this feature. For these reasons, I don't think we need any pref for it. I'm not sure whether it is worth a "Intend to ship", but I guess not.

[1] https://lists.w3.org/Archives/Public/www-style/2015Mar/0290.html
[2] https://lists.w3.org/Archives/Public/www-style/2015Mar/0304.html
(Assignee)

Updated

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

Comment 5

4 years ago
Comment on attachment 8579721 [details] [diff] [review]
patch 3 - reftests

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

::: layout/reftests/w3c-css/submitted/lists-3/list-style-type-string-001-ref.html
@@ +6,5 @@
> +  <link rel="author" title="Xidorn Quan" href="mailto:quanxunzhen@gmail.com">
> +  <style>
> +    ol, ul { list-style: none; }
> +    li::before { content: "# "; }
> +    span { line-height: 0; }

This line is useless. I've removed it locally.
Attachment #8579717 - Flags: review?(dbaron) → review+
Comment on attachment 8579720 [details] [diff] [review]
patch 2 - handle string value on list-style-type

Maybe it's better for AnonymousCounterStyle::GetSuffix to call
IsSingleString() rather than checking mSingleString directly?

nsRuleNode.cpp:

>+      nsString content;
>+      typeValue->GetStringValue(content);
>+      list->SetListStyleType(content, new AnonymousCounterStyle(content));

"content" seems like a bad name for this variable; such a name
would usually refer to an nsIContent*.  Maybe just "str"?

Also, I'm curious why you pass content/str as the first argument
to SetListStyleType rather than passing NS_LITERAL_STRING("")
as you do for symbols().  It doesn't look like you depend on it
anywhere, but it seems like it might make more sense to use the
empty string here.

r=dbaron with that
Attachment #8579720 - Flags: review?(dbaron) → review+
Comment on attachment 8579721 [details] [diff] [review]
patch 3 - reftests

I guess it's the span { line-height: 0; } that you said you will remove?

r=dbaron
Attachment #8579721 - Flags: review?(dbaron) → review+
(Assignee)

Comment 9

4 years ago
Ah... Why does "font-variant-numeric" also affect symbols and space characters...
(Assignee)

Comment 10

4 years ago
Adding "font-variant-numeric: tabular-nums;" to the ref makes it pass the test:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45fd90e05b89

But I wonder whether we can add it in a test submitted to W3C. I've sent an email to www-style to propose adding this rule to ::marker by default to make the spec match what we currently do: https://lists.w3.org/Archives/Public/www-style/2015Mar/0389.html

If the editors accept this, I'll continue landing the current patches.
https://hg.mozilla.org/mozilla-central/rev/00b852893926
https://hg.mozilla.org/mozilla-central/rev/0a3c37c82aa0
https://hg.mozilla.org/mozilla-central/rev/3b8e79ee4339
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Release Note Request (optional, but appreciated)
[Why is this notable]: A useful layout change for developers
[Suggested wording]:  List-style-type now accepts a string value
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/CSS/list-style-type

(Assuming that link is where developer documentation will be added)
relnote-firefox: --- → 39+
You need to log in before you can comment on or make changes to this bug.