-
Notifications
You must be signed in to change notification settings - Fork 22.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSS Counters - Explained the differences in browsers behavior with respect to CSS Counters. #13513
Conversation
|
||
## Using counters | ||
Counters are essentially variables managed by CSS, not only for their own counters, but also for ordered list numbers, which can be increased or decreased by arbitrary values according to CSS rules, and can be used to define multiple named counters, or to manipulate the list-item counters that is standard generated as ordered lists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence needs to be split into much smaller parts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bb3d763 fix it.
To use a counter it must first be initialized to a value with the {{cssxref("counter-reset")}} property. | ||
The counter's value can then be increased or decreased using {{cssxref("counter-increment")}} property. | ||
The current value of a counter is displayed using the {{cssxref("counter()")}} or {{cssxref("counters()")}} function, typically within a [pseudo-element](/en-US/docs/Web/CSS/Pseudo-elements) {{CSSxRef("content")}} property. | ||
Counters can be used by using {{cssxref("counter-reset")}}, {{cssxref("counter-increment")}} and {{cssxref("counter-set")}} properties, and {{cssxref("counter()")}} and {{cssxref("counters()")}} functions as a value of {{cssxref("content")}} property. For reversed counters, the `reversed()` function can also be used as a value of `counter-reset` property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Counters can be used by using" - hard to read. Maybe counters can be used by "applying" ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9dd98a6 fix it.
files/en-us/web/css/css_counter_styles/using_css_counters/index.md
Outdated
Show resolved
Hide resolved
I'm not reviewing this properly as I'm not an expert. That said, I do prefer this structure for the using guide. Some of the English could be improved a little - in particular if a sentence ends up being a 3 or 4 line paragraph split it into short distinct sentences. One thing I particularly like is that I never understood why |
…nters can be used by applying" (mdn#13513)
Thank you for your comnent. It is very helpful for me. I have tried to fix it, but what do you think? In addition to the inline remarks, I also add a sentence about |
Hi @debiru Thank you. I will take another look tomorrow when I am fresh. I am also hoping someone with more expertise around these APIs can also review. Though I do have some sort of understanding, since I did the recent iteration on reversed counters. This is why I am very happy you have updated the structure, because this is better. |
Thank you for your message. I have done my own self-review and made minor fixation. If you want to see your changes immediately, you can preview them at your local environment using |
- {{cssxref("counter()")}} and {{cssxref("counters()")}} functions | ||
- {{cssxref("content")}} property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was content removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally, Using_CSS_counters
, counter-reset
, conuter-increment
, counter-set
, counter()
and counters()
descriptions were inconsistent (did not normalized).
The content
property was excluded from the counter-reset
and counter-set
pages because it is not directly related to the CSS Counter. On the other hand, it is kept in the counter()
and counters()
functions.
If you have a suggestion that it should be written, I will add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bdaa073 added content
link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only just started the review. Only one question on the see also edits, but need to read the whole "using CSS counters", which I haven't had time to finish yet.
files/en-us/web/css/css_counter_styles/using_css_counters/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/css/css_counter_styles/using_css_counters/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/css/css_counter_styles/using_css_counters/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/css/css_counter_styles/using_css_counters/index.md
Outdated
Show resolved
Hide resolved
Thank you for reviewing this. Especially, English sentences may have many mistakes because I am Japanese and not good at English. I know this may be difficult since the document is long, but it would be helpful if you could review it. Once the review is complete I will make the revisions. |
Thank you for taking the time to re-work all the counter documentation. I think the main reason for your edits to the using CSS counters document was to highlight:
This would be a great addition to BCD (browser compatibility data), and can be addressed as a note or warning in the documentation, or even subsection with a header. BCD data for CSS properties are located at https://github.com/mdn/browser-compat-data/blob/main/css/properties/ MDN documentation is geared towards both experienced developers and people just learning the topic with little to no experience. I think the original is easier to follow for those who have little experience. I encourage you to revert back to the original "Using CSS counter", amending the content to reflect modern specifications and correct implementation, then adding a note or section on how to ensure non-supporting browsers get the right results in a way that can easily be removed when Safari updates their implementation. Also:
Your English is excellent! Thank you for contributing. |
…x.md Co-authored-by: Estelle Weyl <[email protected]>
…x.md Co-authored-by: Estelle Weyl <[email protected]>
…x.md Co-authored-by: Estelle Weyl <[email protected]>
…x.md Co-authored-by: Estelle Weyl <[email protected]>
I know BCD. This issue is not about whether or not the specification is supported. The issue was the lack of information about how Firefox 82 or later differs from other browsers in behavior that is not specified in the specification.
This statement was inappropriate because it was information at a certain point in time. It should have read something like "If counter-set is not supported, counter-reset must be used. This paragraph is unnecessary and should be deleted. refs. fdd9b24
Please forget about the Safari description. That was inappropriate. Would a rewritten document be difficult to read for someone without knowledge? I do not know for whom the MDN documentation should be written. |
Long format documentation which explains how to implement things, is written with the learner in mind, at a 3rd grade to 8th-grade reading level, always keeping people in mind who may have cognitive and learning differences. |
Hi @estelle
I threw away the previous document and rewrote it anew. I have rewritten the document based on the structure of the original document, filling in the missing content. I believe that the rewrite has been done while maintaining the level of the intended audience. Would this still be considered something of a problem? I would like to hear your honest opinion. Also, suggestions for errors in the English language are welcome. |
…nter() and counters() (mdn#13513)
I don't know the process for merge requests in this repository, but is it possible to get anyone to review it as it is? Please let me know what I need to do. |
Thanks for the ping @debiru . Sorry for the delay in review - we're all busy on different things and counters a kind of hard (at least in my opinion). I'll see if I can drag in some expertise. Feel free to ping again if you don't see any response by mid next week. |
Thank you for your response. It is perfectly acceptable for the response to be delayed due to your busy schedule. I and other contributors have no knowledge of the situation. We were concerned that the situation would be left for six months or more without any response. It is bad for a contributor's motivation to be left in the dark about the reviewer's or core committer's situation. In this case, since the situation was explained to us, there was no problem. |
Hi @debiru The best way to get this reviewed would be to separate the factual updates and structural improvements. As @estelle suggested here #13513 (comment) : adding a warning and update to BCD is relatively clearly of value, but because it is constrained it is easy to review. I would start again from scratch and just add that information. The structural changes are always going to be harder to get review for. Keeping them separate at least doesn't block the key content going in. |
I would like to confirm, are you saying this after reading my second rewrite of the document? My first rewrite was indeed a mixture of structural changes and information additions. I would write about adding information to the BCD if I could, but there were two problems.
Therefore, in my second rewrite, I added additional information in additional sections based on the improved structure.
By the way, what does scratch mean? Is it a blank page? Or is it the current version, before I edited it? If you are asking me to make further modifications, would it be possible for you to advise me more specifically? I apologize for the inconvenience caused by my inability to help you. |
Hi @debiru
No. I have not reread the second rewrite - I scanned it and just saw "lots of change". I haven't given it a thorough review because there is no point if Estelle isn't happy. I think you are saying that your rewrite is not so much "lots of change", but the minimal change needed to explain the browser differences. Is that right?
It means start from the page "as it is now on MDN" and make the minimal changes needed to record the changed behaviour. Let's see what @estelle says. Generally the less change there is from the original, the easier it is for us to review the change. If she says nothing I'll look at her comments tomorrow and think about how I might do the changes. |
My first rewrite was certainly difficult to get through the review. In my second rewrite, I have taken note of what @estelle said and added the information while keeping the current structure.
I have rewritten it from two perspectives.
The result of this work is the current difference. I don't mean to add any more extraneous explanations than necessary. If the second rewrite does not pass review, then I will consider a third rewrite as you said. |
ol { counter-reset: num list-item; } | ||
``` | ||
|
||
Firefox 68 and later, which do not apply implicit `counter-reset: list-item`, are affected by this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to go into mdn/browser-compat-data#15666 or a subsequent patch.
I don't understand it though - can you point me to an issue number or implementation change?
2-4-2. (2-2-1) | ||
``` | ||
|
||
Note that before {{cssxref("counter-set")}} was introduced, this case was implemented using {{cssxref("counter-reset")}}. Firefox 82 and later, which implement strict scoping, are affected by this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually confusing. It is hard to work out what is expected and what is not.
I think we need to clarify how counter-set
works by contrast. You may have defined strict scoping too. I'll come back to this once the BCD stuff works through.
@debiru Thanks for your patience. I'm going to try work through this the way that I always sort out these problems - I can't deconstruct what you have done. Having "some" understanding of the issues now I think I agree with Estelle - there is too much going on in this PR to work out whether the changes are helpful or not. I think the information will definitely useful, but we should integrate it carefully. EDIT -note, I might just be a bit stupid of course :-). I tend to work through these things methodically. The first step in that is to capture all browser-specific behaviour in browser compatibility data. Generally we try and avoid having specific comments about browser behaviors in the MDN text itself if at all possible. So as a first step I have created mdn/browser-compat-data#15666 which captures the incompatibility with the spec for all browsers (and FF prior to FF82) related to how The reason I do this is that this ensures that there is a clear understanding of what the breaks are and where. The BCD team are pretty careful about that, so the issue won't get through if the problem isn't clearly stated. When that's done, I would update the
Then I'd go to the "Using" doc and update that as necessary. That's what Estelle means by a discrete change - we're not trying to make the whole thing better, we're trying to isolate the specific issues first. Anyway, I see #13513 (comment) says that there is some behaviour that Firefox 68 and later have related to the implicit |
Thanks for your explanation.
OK. I will proceed with the third rewrite to make really small differences. Then I will strike out all edited differences up to the my second rewrite. I also understood that compatibility information should not be included in the text. First, let me consider updating the BCD table. I will comment so far once. Thanks.
|
Sounds like a great plan.
|
@hamishwillee I appreciate it.
That's certainly true. My rewrite was triggered by the
I wrote these bug reports, but both were my mistakes, and the Firefox implementation was correct. I decided to do a rewrite of the MDN to make these facts known to many people. However, my lack of knowledge of MDN's documentation rewrite practices resulted in this Pull-Req of the "Using" document. I really appreciate you taking a look at this Pull-Req. |
Note, not forgotten this. Just still below my other priorities. |
I think this Pull-Req should be rejected once. It would be simpler to create a new diff, ignoring the changes made in this Pull-Req, even if we will modify this pages. Conflicts have also been caused by other modifications. It is time-consuming to resolve conflicts again and again. |
Summary
Rewritten the Using CSS counter systematically.
And added a link for "Using CSS Counters" and normalize "See also" links.
Motivation
See #13293
Supporting details
Without losing the content of the original description, it has been rewritten in more detail.
In particular, the behavior of Firefox 82 and later was explained.
Current version:
Related issues
See #13293
Metadata