Hixie (not reading bugmail) Reporter
	Description • 26 years ago

You do not support the :before and :after pseudo-elements, which are part of
CSS2 generated content.

This is defined in the spec:
http://www.w3.org/TR/REC-CSS2/generate.html

Generated content isa prerequisite to correctly implementing Q (see bug #1061)
in the ua stylesheet, and would also be useful to QA in testing attribute
support (in addition, of course, to being useful in its own right on the web!).

	Hixie (not reading bugmail) Reporter
	Comment 1 • 25 years ago

Test cases for generated content...
...with text:
  http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/content/1.html
...with quotes:
  http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/content/2.html
...with url()s:
  http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/content/3.html
...and with attr()s:
  http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/content/4.html

	Peter Linss
	Comment 2 • 25 years ago

The content properties are now in the style contexts. Assign to rickg for the
counter support when you're done...

	troy
	Comment 3 • 25 years ago

Thanks Peter

	troy
	Comment 4 • 25 years ago

Rick, assigning to you for counter support. I created a new bug (marked REMIND)
concerning nested quotes (currently not implemented)

	Hixie (not reading bugmail) Reporter
	Comment 5 • 25 years ago

Nested quotes support is currently waiting in bug 1061.
Note: A discussion on how to implement display:list-item (a css1 task) is
currently playing itself out in bug 4522. That bug would be easy to solve if
generated counter was implemented.

	Hixie (not reading bugmail) Reporter
	Comment 6 • 25 years ago

*** Bug 9840 has been marked as a duplicate of this bug. ***

	Hixie (not reading bugmail) Reporter
	Comment 7 • 25 years ago

When this is being implemented, don't forget that:
# When the 'display' property has the value 'marker' for content generated by
# an element with 'display: list-item', a marker box generated for ':before'
# replaces the normal list item marker.
   -- http://www.w3.org/TR/REC-CSS2/generate.html#markers

Also, if counter() and counters() are not implemented, then the entire 'content'
declaration in which they occur should be ignored. That is so that authors can
easily work around this lack of feature. NOTE! THIS IS CURRENTLY A BUG!

i.e., currently, the following:

   SPAN:before { content: "works"; }
   SPAN:before { content: counter(x) "bug!"; }

...displays "bug!". It should not, since counters are not supported. It should
display "works". This is due to the forward-compatability parsing rules: if a
feature is not supported, then the declaration should be ignored. Rick, if we
are not going to support CSS2 counters, then I suggest we file a bug against
Peter so that he ignores 'content' declarations with 'counter's.

	Hixie (not reading bugmail) Reporter
	Comment 8 • 25 years ago

This last issue is now covered by bug 15174. I am marking that bug dependent on
this one, but technically it is an either|or situation.

	Hixie (not reading bugmail) Reporter
	Comment 9 • 25 years ago

*** Bug 14948 has been marked as a duplicate of this bug. ***

	Hixie (not reading bugmail) Reporter
	Comment 10 • 25 years ago

Migrating from {css2} to css2 keyword. The {css1}, {css2}, {css3} and {css-moz}
radars should now be considered deprecated in favour of keywords.
I am *really* sorry about the spam...

	Hixie (not reading bugmail) Reporter
	Comment 11 • 24 years ago

Reopening and moving to Future.

	Hixie (not reading bugmail) Reporter
	Comment 12 • 24 years ago

*** Bug 51393 has been marked as a duplicate of this bug. ***

	Hixie (not reading bugmail) Reporter
	Comment 13 • 24 years ago

*** Bug 57393 has been marked as a duplicate of this bug. ***

	Frank Tang
	Comment 14 • 24 years ago

add pierre and erik to the cc list.

	Boris Zbarsky [:bzbarsky]
	Comment 15 • 24 years ago

*** Bug 70191 has been marked as a duplicate of this bug. ***

	Boris Zbarsky [:bzbarsky]
	Comment 16 • 23 years ago

*** Bug 81381 has been marked as a duplicate of this bug. ***

	Boris Zbarsky [:bzbarsky]
	Comment 17 • 23 years ago

*** Bug 94182 has been marked as a duplicate of this bug. ***

	Jesse Ruderman
	Comment 18 • 23 years ago

Fixing summary to show up on searches for "css counter".

	Madhur Bhatia
	Comment 19 • 23 years ago

any progress with this bug?

	Hixie (not reading bugmail) Reporter
	Comment 20 • 23 years ago

not while it's assigned to nobody... reassigning to default owner.

	Christian Eyrich
	Comment 21 • 22 years ago

Oh, great, after 7 duplicates of this bug, over 3 years gone by and CSS3
comming,  still nobody who cares for counters? Maybe it's a provocation since
Opera 6 supports it ...

	Sören 'Chucker' Kuklau (gone)
	Comment 22 • 22 years ago

#21
> Oh, great, after 7 duplicates of this bug, over 3 years gone by and CSS3
> comming,  still nobody who cares for counters?

It's not like nobody cares about it. I do, but can't fix it as I'm not a coder.

> Maybe it's a provocation since Opera 6 supports it ...

Ugh...

	David Baron :dbaron: Assignee
	Comment 23 • 22 years ago

Speaking of CSS3, counters are likely to work quite differently in CSS3 than
CSS2 since the counter model in CSS2 has serious problems.  (Is it really
implemented *correctly* in Opera?)

	Sören 'Chucker' Kuklau (gone)
	Comment 24 • 22 years ago

#23
> Speaking of CSS3, counters are likely to work quite differently in CSS3 than
> CSS2 since the counter model in CSS2 has serious problems.

Like? (Out of curiosity)

> (Is it really implemented *correctly* in Opera?)

http://www.xs4all.nl/~ppk/css2tests/counter.html says yes. I didn't test it
extensively though.

	Greg K.
	Comment 25 • 22 years ago

Actually, that page appears to be correct in stating that Opera 5 implements
counters.

	Christian Eyrich
	Comment 26 • 22 years ago

Yeah, Opera supports it including nested counters.
Don't know how far and if correctly in really hard environments, but as far I
could see good enough for normal sites.

My first comment should thought of a wake-up to this old bug and as he mailed me
someone is really working on counters.

	Christian Eyrich
	Comment 27 • 22 years ago

4 Testcases with hyperlinks "should be" pics.

	Madhur Bhatia
	Comment 28 • 22 years ago

This bug has been open for over 3 years now. It would be nice to see a 
foreseeable milestone set for this bug. 

-- should this bug not fall under the Style System Component?

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 29 • 22 years ago

for reference:
url to the section of the spec:
http://www.w3.org/TR/REC-CSS2/generate.html#counters

	Marcus Campbell
	Comment 30 • 22 years ago

-> Style System

	David Baron :dbaron: Assignee
	Comment 31 • 22 years ago

Layout is the correct component.  Please leave it there.

	Marcus Campbell
	Comment 32 • 22 years ago

Named counters have been removed in CSS 2.1:
* http://www.w3.org/TR/CSS21/changes.html#q81
* http://www.w3.org/TR/CSS21/generate.html

Does this make the bug FIXED, seeing as the original problem was to implement
:before and :after?

	Kai Lahmann (is there, where MNG is)
	Comment 33 • 22 years ago

no, they are still valid for CSS 2.0 and will be for CSS 3, so it only lowers
the priority for most of the missing CSS properties (except outline)

	Olivier Cahagne
	Comment 34 • 22 years ago

*** Bug 189465 has been marked as a duplicate of this bug. ***

	Michael Lefevre
	Comment 35 • 22 years ago

*** Bug 190268 has been marked as a duplicate of this bug. ***

	Nathan Kurz
	Comment 36 • 22 years ago

Comment #32 says that named counters were removed in the proposal for CSS2.1,
but reading the links provided, I see no such evidence.  The word "counter" is
not mentioned in the changes file at the link provided, and (to my untrained
eye) the section on named counters looks to be identical in the CSS2 rec and
proposed CSS2.1 spec.  

http://www.w3.org/TR/CSS21/generate.html#counters
http://www.w3.org/TR/REC-CSS2/generate.html#counters

Thus, as far as I can tell, this is just as much of a bug as back in 1999.  If
I'm reading the specs wrong, perhaps someone (Marcus?) could clarify comment #32
to avoid future confusion?  Or perhaps since CSS2.1 is still a working draft,
things changed in the interim that comment was made?

	Marcus Campbell
	Comment 37 • 22 years ago

I should have originally linked to the actual draft file but, yes, it seems that
they've added counters back into the latest draft (released on 28th January
2003). I think I remember reading that they added it back in because Opera could
support it.

Previous generated content section:
http://www.w3.org/TR/2002/WD-CSS21-20020802/generate.html

Current generated content section:
http://www.w3.org/TR/2003/WD-CSS21-20030128/generate.html

Strangely, unless I'm missing something, there doesn't seem to be any mention of
this in the changes:
http://www.w3.org/TR/2003/WD-CSS21-20030128/changes.html#q17

	Hixie (not reading bugmail) Reporter
	Comment 38 • 22 years ago

Counters will be in CSS3 regardless. CSS2.1 does not affect whetehr Mozilla will
implement something or not in any way.

	Boris Zbarsky [:bzbarsky]
	Comment 39 • 22 years ago

*** Bug 193927 has been marked as a duplicate of this bug. ***

	Jeff Dickey
	Comment 40 • 22 years ago

So will the counter support be fixed anytime soon?  This *is* an important
feature for sites (probably mostly corporate intranets) that deal with a lot of
outline-numbered documents; it would make cut-and-paste between documents (or
includes of common subdocuments) a he!! of a lot easier to deal with.  Opera 7
does it quite nicely; this has been in the CSS2 spec for over four years now...

	Hixie (not reading bugmail) Reporter
	Comment 41 • 22 years ago

Fixing bugs with existing features, making our performance better, making us
take less disk space and memory, and making us crash less are all considered
more important right now. Having said that, we welcome patches, if you want to
implement it.

	Boris Zbarsky [:bzbarsky]
	Comment 42 • 21 years ago

.

	Alex Vincent [:WeirdAl]
	Comment 43 • 21 years ago

I think HTML 4.01 Strict, XHTML 1.0 Strict and XHTML 1.1+ would benefit from
this bug being fixed.

Consider:

<ol type="number" start="3">
  <li>Three</li>
  <li>Four</li>
</ol>

The start attribute in HTML 4.01 is deprecated.  Thus, the CSS counter-reset
property is apparently the only way for valid HTML 4.01 Strict documents to have
numbered lists that don't start at 1.

	Bill Mason
	Comment 44 • 21 years ago

*** Bug 213703 has been marked as a duplicate of this bug. ***

	Bill Mason
	Comment 45 • 21 years ago

*** Bug 214871 has been marked as a duplicate of this bug. ***

	David Baron :dbaron: Assignee
	Comment 46 • 21 years ago

->style system, for lack of a better component for what probably centers around
frame construction

	Anne (:annevk)
	Comment 47 • 21 years ago

Is there a separate bug for:

 p::before{
  content:url(data.txt);
 }

? Simple tests: http://www.annevankesteren.nl/test/css/p/content/ (only the
second 'works' in Mozilla, the first depends on another bug/CSS3)

	David Baron :dbaron: Assignee
	Comment 48 • 21 years ago

comment 47 is unrelated to this bug.  comment 47 is about the css3 extensions to
the content property, but the assertions made in the are probably not valid anyway.

	Anne (:annevk)
	Comment 49 • 21 years ago

Why is that CSS3 (please don't start about the ::, with : it doesn't work
either)? <http://www.w3.org/TR/CSS21/generate.html#propdef-content>

><uri>
>    The value is a URI that designates an external resource. If a user agent
>    cannot support the resource because of the media types it supports, it must
>    ignore the resource.

I think Mozilla supports .txt so it should so the file. I just wanted to know if
this is the same bug, 'cause the third test case from comment 1 demonstrates
this with a .html file.

	David Baron :dbaron: Assignee
	Comment 50 • 21 years ago

This bug is about counters.  Please don't clutter it up with unrelated comments.
 If counters are implemented, this bug will just be marked fixed, and the
comments will be ignored.

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 51 • 21 years ago

*** Bug 230681 has been marked as a duplicate of this bug. ***

	Tobias Herp
	Comment 54 • 21 years ago

Comment on attachment 138874 [details]
Advanced counter example, using the counters() function

sorry, the same one uploaded twice

	Boris Zbarsky [:bzbarsky]
	Comment 55 • 20 years ago

I think an approach similar to that of bug 24861 could be taken here (with use
of one list per named counter to keep track of the current value or something).
 David, does that seem like a viable approach to you?

	David Baron :dbaron: Assignee
	Comment 56 • 20 years ago

Similar, but not identical.  Counters are easier since they are limited to a
subtree, but harder since there are many of them.  We definitely want to exploit
the fact that they're limited to a subtree for finding the right counter to use,
but yes, each counter could probably be stored in a similar way to the way that
patch stores quotes (and hopefully even share some code).

	David Baron :dbaron: Assignee
	Comment 57 • 20 years ago

This improves the parsing code and makes a bunch of other changes such that
"all" that is left is the actual frame construction code for the counters and
the integration / replacement of the existing block frame marker code.	Then
again, the implementation of markers and the conversion of list items to use
them could wait until a second patch.

	David Baron :dbaron: Assignee
	Comment 58 • 20 years ago

This adds some merging of the counters and quotes code that may or may not be
appropriate.  I want to get some spec clarifications on a few issues before
proceeding (the current spec is quite vague, and what Opera implements doesn't
quite make sense).  There's also a big "XXX XXX" comment that probably means
I'll need to undo the code merging.

	David Baron :dbaron: Assignee
	Comment 59 • 20 years ago

This is what I have in my tree now.  A bunch of the new stuff is just rough
sketches and probably isn't right.  But I'm not planning to work on it more
until the spec is (a) clear and (b) sensible regarding interaction of
counter-{increment,reset} on an element and on its :before pseudo-element, and
likewise regarding the scoping of the counters for the same distinction
(element vs. its :before).

	Sekundes
	Comment 60 • 20 years ago

SInce the clarification of the spec is a long way to go, could you check in the
current implementation as -moz-counter-* stuff?

	Worontzoff Philippe
	Comment 61 • 20 years ago

Anything new ? It would be great to fix this bug for FireFox 1.0. ;-)

	mozbugs3
	Comment 62 • 20 years ago

(In reply to comment #61)
> Anything new ? It would be great to fix this bug for FireFox 1.0. ;-)

Oh, come on, it's not even been open for six years yet. You can't expect David
to work this fast. :)

	Worontzoff Philippe
	Comment 63 • 20 years ago

After reseaving a private e-mail, I would like to ask people not to take my
previous comment seriously.

This was my way to say "hey don't forget this bug".

More over, I admit that I read too quickly the comment saying :
"I want to get some spec clarifications on a few issues before
proceeding (the current spec is quite vague, and what Opera implements doesn't
quite make sense).".

	David Baron :dbaron: Assignee
	Comment 68 • 20 years ago

The problem with this patch (other than not quite working yet for the static
case) is that it uses the wrong data structures.  It would be much easier to
handle the dynamic cases if I use a single linked list for each counter name,
rather than one per scope.  The scopes should just be elements in the list
(probably just "start scope", not push/pop).  Each list element should have a
pointer to its appropriate "start scope" element (which has the parent number).

	David Baron :dbaron: Assignee
	Comment 69 • 20 years ago

This approach seems better.  This patch passes all but one of my static
testcases (one that Opera also fails, but differently).  I need to reexamine
that testcase (t1204-counter-scope-00-c.xht) and work on the dynamic change
handling, which should be relatively easy with this approach.

	David Baron :dbaron: Assignee
	Comment 70 • 20 years ago

The dynamic change handling is now implemented, although completely untested.

	David Baron :dbaron: Assignee
	Comment 73 • 20 years ago

I need to check that this code behaves reasonably when pseudo-elements other
than :before or :after are involved.

	David Baron :dbaron: Assignee
	Comment 75 • 19 years ago

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

There are still some open issues before the WG (well, one I want to revisit, at
least), but the necessary change to the patch is tiny, so I may as well request
review.

	David Baron :dbaron: Assignee
	Comment 76 • 19 years ago

Merged to trunk, with a bug I noticed (accidentally removed a |break|) fixed as
well.

	Adam Messinger
	Comment 77 • 19 years ago

With Mozilla suite being end-of-lined, should this bug be moved to Firefox?

	Anne (:annevk)
	Comment 78 • 19 years ago

No. This is a "bug" in the Mozilla Platform (the Core product). Fixing this bug
will make this feature be supported on all products based on the Mozilla
Platform. Firefox is one such product.

	Adam Messinger
	Comment 79 • 19 years ago

(In reply to comment #78)
> No. This is a "bug" in the Mozilla Platform (the Core product). Fixing this bug
> will make this feature be supported on all products based on the Mozilla
> Platform. Firefox is one such product.

Thanks for the clarification, Anne. I was making my assumption based on the
"Target Milestone" field, but from now on I'll know to pay more attention to
"Product" instead.

	David Baron :dbaron: Assignee
	Comment 80 • 19 years ago

This cleans up some things and changes the behavior a little -- I found some
problems with what the working group agreed to.  I've updated my tests as well.

	David Baron :dbaron: Assignee
	Comment 81 • 19 years ago

This fixes 'none', 'disc', 'circle', and 'square' when used with counters to at
least do something reasonable (although it uses glyphs instead of the special
code we have for list bullets).

	David Baron :dbaron: Assignee
	Comment 82 • 19 years ago

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

> PresShell::NotifyDestroyingFrame(nsIFrame* aFrame)
> {
>+  // XXX Should this be inside mIgnoreFrameDestruction test?
>+  mFrameConstructor->NotifyDestroyingFrame(aFrame);
>+

Oh, and I answered my own question here (and moved it inside).	The answer is
yes since the only thing that sets it is nsFrameManager::Destroy, and right
before we call that we call mFrameConstructor->WillDestroyFrameTree, which
clears the counter and quote lists.

	David Baron :dbaron: Assignee
	Comment 83 • 19 years ago

Merged to trunk, plus what I mentioned in comment 82.

	David Baron :dbaron: Assignee
	Comment 84 • 19 years ago

Although I realized I should probably rename nsCSSValue::List to nsCSSValue::Array.

	David Baron :dbaron: Assignee
	Comment 85 • 19 years ago

This fixes a bunch of bugs in the nsCSSValue and related changes, does the
renaming I mentioned in the previous comment, and adds computed value support
too.

	David Baron :dbaron: Assignee
	Comment 86 • 19 years ago

I realized serialization of counters() isn't quite right -- I should probably
store the params in the order specified so they don't get reordered, and just
deal with that in the code that uses the nsCSSValue::Array.

	Boris Zbarsky [:bzbarsky]
	Comment 87 • 19 years ago

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

>Index: layout/base/nsCSSFrameConstructor.cpp

>@@ -6818,24 +6864,28 @@ nsCSSFrameConstructor::InitAndRestoreFra

>+  if (mCounterManager.AddCounterResetsAndIncrements(aNewFrame)) {

Should probably assert that aNewFrame is a first-in-flow here.	Otherwise
things will get a bit confused, I think.

Also, there are a few places in the frame constructor that don't call
InitAndRestoreFrame but may want to affect counters (the replacement wrapper
frame for <img> elements and letter frames, to be precise)...

>Index: layout/base/nsCSSFrameConstructor.h

>+  void NotifyDestroyingFrame(nsIFrame* aFrame);

Want to add a comment here about when this method is to be called (for all
frame destructions happening before Destroy(), basically)?

>Index: layout/base/nsCounterManager.cpp

>+#include "nsCounterManager.h"

>+// The text that should be displayed for this quote.
>+void
>+nsCounterUseNode::GetText(nsString& aResult)

s/quote/counter/ ?

>+    for (PRInt32 i = stack.Count() - 1;; --i) {
>+        nsCounterNode *n = NS_STATIC_CAST(nsCounterNode*, stack[i]);
>+        stack.RemoveElementAt(i);

Why bother removing it?   The array will go out of scope and go away when we
return anyway...

>+nsCounterList::SetScope(nsCounterNode *aNode)

I'm having a hard time following the logic in this method.... Could you maybe
explain the basic idea of the algorithm up front before diving into the code?

>+nsCounterManager::AddResetOrIncrement(nsIFrame *aFrame, PRInt32 aIndex,

>+    counterList->Insert(node);
>+    if (!counterList->IsLast(node)) {
>+        return PR_TRUE;
>+    }
>+    node->Calc(counterList);

I guess we don't need to call Calc() in the !IsLast() case because caller
should recalc the whole list?  If so, worth documenting, I think.

>+nsCounterManager::CounterListFor(const nsSubstring& aCounterName)

>+        counterList = new nsCounterList;

|new nsCounterList()| is probably clearer (add the parens).

>Index: layout/base/nsCounterManager.h

>+struct nsCounterNode : public nsGenConNode {
>+    enum Type { RESET, INCREMENT, USE };

Add a comment saying that RESET and INCREMENT correspond to counter-reset and
counter-increment respectively and that USE corresponds to a counter() or
counters()?

> +    nsCounterNode(nsIFrame* aPseudoFrame, PRInt32 aContentIndex, Type aType)

What's aContentIndex here?  And why aPseudoFrame?  Those names make sense for
USE nodes, but not so much for INCREMENT and RESET nodes.....  Some
documentation on what those args mean would be good.

>+    // mScopeStart points to the node (usually a RESET, but not in the
>+    // case of an implied 'counter-reset' that created the scope for
>+    // this element (for a RESET, its outer scope).

Missing ')' after "'counter-reset'"?  What does "outer scope" mean, exactly?
It's not defined in the spec that I can see, and it's not really defined in the
code.... Worth explaining in more detail, perhaps.  I'm guessing this means
that for RESET nodes this points to the scope right outside the scope the RESET
creates.

>+    // mScopePrev points to the previous node that is in the same scope,
>+    // or for a RESET, the previous node in the scope outside of the
>+    // reset.
>+
>+    // May be null for all types, but only when mScopeStart is also
>+    // null.

Because otherwise it could always point to the mScopeStart RESET node?	May be
worth saying so explicitly.

>+struct nsCounterUseNode : public nsCounterNode {

>+    nsCounterUseNode(nsCSSValue::Array* aCounterStyle, nsIFrame* aPseudoFrame,
>+                     PRUint32 aContentIndex, PRBool aAllCounters)

Again, please document the args?  In particular what is aCounterStyle expected
to contain, exactly?

Alternately, and perhaps better, document mCounterStyle and just say here that
aCounterStyle should be what we store in mCounterStyle.

>+    // The text that should be displayed for this quote.
>+    void GetText(nsString& aResult);

s/quote/counter/

>+    nsCounterChangeNode(nsIFrame* aPseudoFrame,

It's not really necessarily a pseudoframe, is it?

>+class nsCounterList : public nsGenConList {

>+    void SetScope(nsCounterNode *aNode);

Document what that does?

>+    void RecalcAll();

And this.

>+/**
>+ * The counter tree can be used to look up 
>+ */

Look up what?

>+    PRBool AddResetOrIncrement(nsIFrame *aFrame, PRInt32 aIndex,
>+                               const nsStyleCounterData *aCounterData,
>+                               nsCounterNode::Type aType);

What's aIndex here?  What does this function do?

>Index: layout/base/nsGenConList.cpp
>+nsGenConList::NodeAfter(const nsGenConNode* aNode1, const nsGenConNode* aNode2)
>+  if (pseudoType1 == 0 || pseudoType2 == 0) {
>+    if (content1 == content2) {
>+      NS_ASSERTION(pseudoType1 != pseudoType2, "identical");
>+      return pseudoType2 == 0;
>+    }
>+    // We want to treat an element as coming before its :before (preorder
>+    // traversal), so treating both as :before now works.
>+    pseudoType1 = -1;
>+    pseudoType2 = -1;

I'm not sure I follow.	Say we have the following markup:

  <div><span /></div>

and content1 is the div, content2 is the span, with pseudoType2 == 0.  Then we
get into this case.  But the return value should clearly depend on whether
we're talking about the ::before or ::after of the <div>.  At the same time,
this code will call DoCompareTreePosition with the same arguments for both of
these cases. That can't be right.

>Index: layout/base/nsGenConList.h

>+struct nsGenConNode : public PRCList {
>+  // The wrapper frame for all of the pseudo-element's content.  This
>+  // frame generally has useful style data and has the
>+  // NS_FRAME_GENERATED_CONTENT bit set (so we use it to track removal),
>+  // but does not necessarily for |nsCounterChangeNode|s.
>+  nsIFrame* const mPseudoFrame;

Maybe mFrame, then?  Since it's not always a pseudo-frame?  And document what
it means in the struct decls for the specific node types?

>+  // Index within the list of things specified by the 'content' property,
>+  // which is needed to do 'content: open-quote open-quote' correctly.

And to do counters right too, no?

Also, for counter increment/reset this isn't the index inside 'content' at all,
is it?	Perhaps just call it mIndex and document what it means for different
node types in the struct decls for those types?

>+    NS_ASSERTION(aContentIndex <
>+                   PRInt32(aPseudoFrame->GetStyleContent()->ContentCount()),
>+                 "index out of range");
>+    // We allow negative values of mContentIndex for 'counter-reset' and
>+    // 'counter-increment'.

And this assert is meaningless for those types of nodes, right?  Perhaps the
assert should be moved down into the relevant node type constructors?

>+    NS_ASSERTION(aContentIndex < 0 ||
>+                 aPseudoFrame->GetStyleContext()->GetPseudoType() ==
>+                   nsCSSPseudoElements::before ||
>+                 aPseudoFrame->GetStyleContext()->GetPseudoType() ==
>+                   nsCSSPseudoElements::after,
>+                 "not :before/:after generated content and not counter use");

"Counter change", you mean?  Counter use _does_ require generated content...

>+    NS_ASSERTION(aContentIndex < 0 ||
>+                 aPseudoFrame->GetStateBits() & NS_FRAME_GENERATED_CONTENT,
>+                 "not generated content and not counter use");

Again, "counter change".

>+  virtual ~nsGenConNode() {} // XXX Avoid, perhaps?

Don't really see how we can, since we have subclasses with members that get
deleted in destructors and we want to delete them through the nsGenConNode
pointer....

>Index: layout/generic/nsBulletFrame.cpp

>+nsBulletFrame::GetListItemText(const nsStyleList& aListStyle,

>+  PRBool success =
>+    AppendCounterText(aListStyle.mListStyleType, mOrdinal, result);

Before that, assert that aListStyle.mListStyleType isn't one of
DISC/CIRCLE/SQUARE?  For those, the boolean that function returns is
undefined... (perhaps it would be better to define the boolean too, to prevent
compiler warnings?)

>Index: layout/style/nsCSSDeclaration.cpp

Like you said, the serialization needs to be fixed up to deal with the original
array order (mostly a note to myself so I remember to check it in new patch
versions).  The code in nsCounterUseNode::GetText will need fixing too.

>Index: layout/style/nsCSSParser.cpp
>+PRBool CSSParserImpl::GetNonCloseParenToken(nsresult& aErrorCode, PRBool aSkipWS)
>+{
>+  if (!GetToken(aErrorCode, aSkipWS))
>+    return PR_FALSE;

Do we want to report the EOF error via error reporting here?  I think we do... 
And perhaps an UNEXPECTED_TOKEN error if the token is a paren?

>-#ifdef ENABLE_COUNTERS
>     if (ParseCounter(aErrorCode, aValue)) {
>       return PR_TRUE;
>     }
>-#endif
>     return PR_FALSE;

How about just:

  return ParseCounter(aErrorCode, aValue);

?  Seems clearer to me.....

> PRBool CSSParserImpl::ParseCounter(nsresult& aErrorCode, nsCSSValue& aValue)

>+  if (!GetNonCloseParenToken(aErrorCode, PR_TRUE) ||
>+      eCSSToken_Ident != mToken.mType) {
>+    SkipUntil(aErrorCode, ')');

I think we want to REPORT_UNEXPECTED_TOKEN here for the case when
GetNonCloseParenToken succeeds but the resulting token is not an ident.

>+    // get mandatory string

Worth documenting that this is the separator string from the spec?

>+    if (!ExpectSymbol(aErrorCode, ',', PR_TRUE) ||
>+        !(GetNonCloseParenToken(aErrorCode, PR_TRUE) &&
>+          eCSSToken_String == mToken.mType)) {
>+      SkipUntil(aErrorCode, ')');

Again, worth having some error reporting here....

>+    if (!success) {
>+      SkipUntil(aErrorCode, ')');

Again, error reporting.

>+  val->Item(1).SetIntValue(type, eCSSUnit_Enumerated);

And note to self that this should be fixed up to deal with serialization.

>+  if (!ExpectSymbol(aErrorCode, ')', PR_TRUE)) {
>+    SkipUntil(aErrorCode, ')');

And again, error reporting.

> PRBool CSSParserImpl::ParseCounterData(nsresult& aErrorCode,
>+    if (ident->LowerCaseEqualsLiteral(sv->str)) {
>+      if (ExpectEndProperty(aErrorCode, PR_TRUE)) {
...
>+      }
>+      return PR_FALSE;

Does this need error reporting?

>+    if (!GetToken(aErrorCode, PR_TRUE) || mToken.mType != eCSSToken_Ident) {
>+      break;

Error reporting?

>+    nsCSSCounterData *data = *next = new nsCSSCounterData();

I can't quite figure out which constructor gets used for data->mCounter and
data->mValue when you do this.... Is it ok that you're not really initializing
data->mValue in some cases?  Would it make more sense to explicitly init it to
0?

>Index: layout/style/nsCSSValue.cpp
>+nsCSSValue::nsCSSValue(nsCSSValue::Array* aValue, nsCSSUnit aUnit)
>+  : mUnit(aUnit)

Asssert in the method that aUnit is valid array unit?

> nsCSSValue::nsCSSValue(const nsCSSValue& aCopy)
>   : mUnit(aCopy.mUnit)

Want to factor out the shared code from here and operator= so we don't have to
keep changing both places every time?  Separate bug fine for this if you want.

>+void nsCSSValue::SetArrayValue(nsCSSValue::Array* aValue, nsCSSUnit aUnit)
>+{
>+  Reset();
>+  mUnit = aUnit;

Assert that aUnit is valid array unit?

>Index: layout/style/nsCSSValue.h

>+  Array* GetArrayValue() const
>+  {
>+    NS_ASSERTION(eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Counters,
>+                 "not a list value");

s/list/array/

>Index: layout/style/nsComputedDOMStyle.cpp

>+nsComputedDOMStyle::GetCounterIncrement(nsIFrame *aFrame,

>+  const nsStyleContent *content = nsnull;
>+  GetStyleData(eStyleStruct_Content, (const nsStyleStruct*&)content, aFrame);
>+
>+  if (content->CounterIncrementCount() == 0) {

Null-check |content|?  You do this later, but you'd crash here if it were
null...

>+nsComputedDOMStyle::GetCounterReset(nsIFrame *aFrame,

Same issue here.


>Index: layout/style/nsRuleNode.cpp

>       if (NS_SUCCEEDED(content->AllocateCounterIncrements(count))) {
>         PRInt32 increment;

That's unused now, no?

>         ourIncrement = contentData.mCounterIncrement;
>         PRInt32 increment;

As is this (you moved it into the while() loop there).

>       if (NS_SUCCEEDED(content->AllocateCounterResets(count))) {
>         PRInt32 reset;

Again, unused.

>         ourReset = contentData.mCounterReset;
>         PRInt32 reset;

And here.

Looks good other than that.

	David Baron :dbaron: Assignee
	Comment 88 • 19 years ago

(In reply to comment #87)
> Should probably assert that aNewFrame is a first-in-flow here.	Otherwise
> things will get a bit confused, I think.

Actually, it should check.

> Also, there are a few places in the frame constructor that don't call
> InitAndRestoreFrame but may want to affect counters (the replacement wrapper
> frame for <img> elements and letter frames, to be precise)...

Not calling InitAndRestoreFrame is a bug, and fixing those bugs shouldn't be in
the scope of this patch.

> >+/**
> >+ * The counter tree can be used to look up 
> >+ */
> 
> Look up what?

Oops, the counter tree doesn't exist anymore, and hasn't since comment 69.

	David Baron :dbaron: Assignee
	Comment 89 • 19 years ago

(In reply to comment #87)
> >+PRBool CSSParserImpl::GetNonCloseParenToken(nsresult& aErrorCode, PRBool
aSkipWS)

> Do we want to report the EOF error via error reporting here?  I think we do... 
> And perhaps an UNEXPECTED_TOKEN error if the token is a paren?

No, it should be the callers job, and I really don't want to bloat the property
value parsers with error reporting anyway except in cases of common errors. 
Likewise on your other comments suggesting error reporting.

	David Baron :dbaron: Assignee
	Comment 90 • 19 years ago

(In reply to comment #87)
> >+    nsCSSCounterData *data = *next = new nsCSSCounterData();
> 
> I can't quite figure out which constructor gets used for data->mCounter and
> data->mValue when you do this.... Is it ok that you're not really initializing
> data->mValue in some cases?  Would it make more sense to explicitly init it to
> 0?

They're nsCSSValue objects, so they have their own constructor (init to
eCSSUnit_Null), so it's fine.  Init to zero or one would be loss of information
for serialization, and would also require knowing in this code which of
'counter-reset' or 'counter-increment' is being parsed.

	David Baron :dbaron: Assignee
	Comment 91 • 19 years ago

Should address bzbarsky's comments.  Passes my tests.

	Boris Zbarsky [:bzbarsky]
	Comment 92 • 19 years ago

> fixing those bugs shouldn't be in the scope of this patch.

Agreed.  Should file them, though.

> I really don't want to bloat the property value parsers with error reporting

OK, fair enough.

Looking at updated patch now.

	Boris Zbarsky [:bzbarsky]
	Comment 93 • 19 years ago

>Index: layout/base/nsCounterManager.cpp

>+nsCounterList::SetScope(nsCounterNode *aNode)

>+    // we're not in the same scope that it is, then it's too deep in the
>+    // frame tree, so we walk up parent scopes until we find something
>+    // appropriate.

I think this could use the following additional comment (feel free to fix
wording as desired, of course):

----------
The scope created by a RESET or implied-reset node is defined to contain the
following content nodes and their descendants:
  1) The content node associated to the RESET
  2) All following siblings of that content node up to, but not including, the
next RESET for the same counter.

Therefore, given a non-RESET aNode and a otherNode that preceds aNode in content
order, aNode will be in the same scope as otherNode if some ancestor (not
necessarily proper) of the content of aNode is either the content of otherNode
or a following sibling of the content of otherNode.  This would mean that this
ancestor's parent is the same as the parent of the content of otherNode.  So
it's enough to check that the parent of the content of aNode is a descendant
(not necessarily proper) of the parent of the content of otherNode.  Note that
for pseudo-elements the content node pointed to by mPseudoFrame is already the
parent of the pseudo-element.
----------

For RESET nodes things seem more complicated, though... Consider the following
markup:

  <p style="counter-reset: mycounter" />
  <div><span style="counter-reset: mycounter" /></div>

and let's trace through this function for the second RESET node.  nodeContent
will end up being the <span>, since aNode is a RESET node.  |start| will be the
previous RESET node, and startContent will end up being the <p>, again because
aNode is a RESET node.  But the <span> is not a descendant of the <p>, so we'll
end up with a null mScopeStart for the second RESET node, which is wrong, as I
understand.

The following case also has a similar issue, but may need a bit more care to get
right:

  parent::before { conter-reset: mycounter; }
  descendant { counter-reset: mycounter; }

  <parent><child><descendant /></child></parent>

So maybe what we want to do for RESET nodes is add the following comment:

----------
If aNode is a RESET node, then aNode is only in the scope of otherNode if a
_proper_ ancestor of the content of aNode is either the content of otherNode or
a following sibling of the content of otherNode.  Therefore, for a RESET aNode
it's enough to check that the grandparent of the content of aNode is a
descendant of the parent of the content of otherNode.

If at any point getting a parent node would give null, that means that we've
reached the top of the tree.  At that point, just use the root node for the
relevant comparison and take special care with pseudo-element children of the root.
----------

This last part of the comment refers to the following case:

  root::before { counter-reset: mycounter }
  child { counter-reset: mycounter }

  <root><child/><root>

In this case the second reset is not scoped inside the first one....

So perhaps the code should perhaps look something like:

  nsIContent *nodeContent = aNode->mPseudoFrame->GetContent();
  if (!aNode->mPseudoFrame->GetStyleContext->GetPseudoType() &&
      nodeContent->GetParent()) {
      nodeContent = nodeContent->GetParent();
  }
  nsIContent *origNodeContent = nodeContent;
  if (aNode->mType == nsCounterNode::RESET && nodeContent->GetParent()) {
      nodeContent = nodeContent->GetParent();
  }

and later

  nsIContent *startContent = start->mPseudoFrame->GetContent();
  if (!start->mPseudoFrame->GetStyleContext()->GetPseudoType() &&
      startContent->GetParent()) {
      startContent = startContent->GetParent();
  }

And then do:

  if (!(aNode->mType == nsCounterNode::RESET &&
        start->mPseudoFrame->GetStyleContext()->GetPseudoType() &&
        startContent == origNodeContent) &&
      nsContentUtils::ContentIsDescendantOf(nodeContent, startContent)) {
      ...
  }

Note the startContent == origNodeContnet check -- this is needed to get the
interaction of ::before and roots and grandkids of nodes right, I think...   The
comment to go with this last part:

----------
If we're placing a RESET node and the start node's frame is a pseudo frame, and
the startContent is the root node, then we could have origNodeContent also be
the root node and nodeContent == origNodeContent.  But in this case, the start
node is scoped to the ::before of the root and aNode is associated to a child of
the root, so |start| is not the scope start for aNode, even though nodeContent
is a descendant of startContent.
----------

>Index: layout/base/nsCounterManager.h

>+    // for |AddConuterResetsAndIncrements| only

"counter", not "conuter".

>Index: layout/style/nsCSSDeclaration.cpp

>+      nsCSSProperty prop =
>+        ((eCSSUnit_Counter <= unit && unit <= eCSSUnit_Counters) && i == 1)
>+        ? eCSSProperty_list_style_type : aProperty;

That's not right for counters(), is it?  For counters() you want i == 2, since
you fixed the parsing code to be order-preserving.

>Index: layout/style/nsCSSValue.cpp

>+void nsCSSValue::SetArrayValue(nsCSSValue::Array* aValue, nsCSSUnit aUnit)

This still needs an assert on the unit.

The diff to nsHTMLCSSStyleSheet that you mailed me looks fine.

	Boris Zbarsky [:bzbarsky]
	Comment 96 • 19 years ago

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

>Index: layout/base/nsCounterManager.cpp

>+nsCounterList::SetScope(nsCounterNode *aNode)
>+    nsIContent *nodeContent = aNode->mPseudoFrame->GetContent();
>+    if (!aNode->mPseudoFrame->GetStyleContext()->GetPseudoType()) {
>+        nodeContent = nodeContent->GetParent();
>+    }

Add an NS_ASSERTION(nodeContent) here, please (since aNode has a previous node,
it can't possibly be for the root and all.

With that, r+sr=bzbarsky.

	David Baron :dbaron: Assignee
	Comment 97 • 19 years ago

(In reply to comment #96)
> Add an NS_ASSERTION(nodeContent) here, please (since aNode has a previous node,
> it can't possibly be for the root and all.

No, it can be for the root:

:root {
  counter-reset: c 0 c 0 c 0;
  counter-increment: c 0 c 0 c 1;
}

The list will start with 6 counters on the root.

However, the null check on startContent is sufficient since if nodeContent is
null then startContent will be too, since startContent is something before
nodeContent.

I'll add an NS_ASSERTION(nodeContent || !startContent, "...") after we get start
content asserting that, as discussed on IRC.

	David Baron :dbaron: Assignee
	Comment 98 • 19 years ago

... except that actually isn't true.

	David Baron :dbaron: Assignee
	Comment 100 • 19 years ago

Checked in to trunk, 2005-04-01 15:07 -0800.

Filed bug 288704 on using counters for list numbering.

Filed bug 288706 on InitAndRestoreFrame not being called all the time.

Filed bug 288707 on duplication of code between nsCSSValue's constructor and
assignment operator.

	David Baron :dbaron: Assignee
	Comment 101 • 19 years ago

Comment on attachment 138875 [details]
Advanced counter example, using the counters() function

This testcase has two problems (at least), FWIW.  First, the comment about the
author's understanding of counter scoping is correct in that the testcase is
wrong:	Hn numbering can't be done with nested counters of the same name, and
requires counters of different names.  Second, it relies on selectors like
"h1.appendix h2:before" which aren't going to match anything, since the h2
isn't a descendant of the h1.

	Anbo Motohiko
	Comment 102 • 19 years ago

(In reply to comment #101)
> (From update of attachment 138875 [details] [edit])
> This testcase has two problems (at least), FWIW.  First, the comment about the
> author's understanding of counter scoping is correct in that the testcase is
> wrong:	Hn numbering can't be done with nested counters of the same name, and
> requires counters of different names.
Your understanding is wrong.

In CSS 2.1 spec:
> Counters are "self-nesting", in the sense that re-using a counter in a child
element automatically creates a new instance of the counter.

http://www.w3.org/TR/CSS21/generate.html#scope


Second point is correct. You must use indirect adjacent combinator '~' instead
of descendant combinator, but this is one of the CSS 3 spec.

	Boris Zbarsky [:bzbarsky]
	Comment 103 • 19 years ago

> Counters are "self-nesting", in the sense that re-using a counter in a child
> element automatically creates a new instance of the counter.

Right.  But note the "child element" part.  Here you're reusing a counter on a
_sibling_ element.

	Anbo Motohiko
	Comment 104 • 19 years ago

(In reply to comment #103)
> > Counters are "self-nesting", in the sense that re-using a counter in a child
> > element automatically creates a new instance of the counter.
> 
> Right.  But note the "child element" part.  Here you're reusing a counter on a
> _sibling_ element.
Oh, that's my mistake.

Reusing counters on child elements does not work (the 'URL'), so I
misunderstood. Sorry!

	Boris Zbarsky [:bzbarsky]
	Comment 105 • 19 years ago

> Reusing counters on child elements does not work (the 'URL')

Which part of it doesn't work?

	Anbo Motohiko
	Comment 106 • 19 years ago

(In reply to comment #105)
> > Reusing counters on child elements does not work (the 'URL')
> 
> Which part of it doesn't work?
It seems to be correct thing. I saw wrong results the other day...


But this URI shows wrong. (Not my site)
http://saiton.net/motors.htm
CSS: http://saiton.net/moto.css

1. ul element does not create an instance of counter (no 'counter-reset'). Its
child list-item elements increment counter 'item', but all of these have '1' value.
In the spec:
> If 'counter-increment' refers to a counter that is not in the scope (see
below) of any 'counter-reset', the counter is assumed to have been reset to 0 by
the root element.

2. ol element creates an instance of counter which have same name as implicit
counter (point 1). Mozilla should create new instance of counter, not override
existence one.


This is a screenshot of this site in Opera 8 beta 3.
http://maguroban.s41.xrea.com/image/saito_opera8b3.png

	Anbo Motohiko
	Comment 107 • 19 years ago

1. No 'counter-reset' in 1st level lists.
2. 'counter-increment' in :before pseudo-element, not list item.

Modifying any of these will show correct result.

	Boris Zbarsky [:bzbarsky]
	Comment 108 • 19 years ago

> I saw wrong results the other day...

The test was wrong (missing a counter-increment) a few days back; it's been
fixed since.

> the counter is assumed to have been reset to 0 by the root element.

This part of the spec is getting changed; what David implemented is the current
agreement in the working group for what it should say.  There are also other
changes, such as what exactly constitutes counter scope, etc, that go hand in
hand with this, by the way.  See the tests at the URL in the URL bar.

	Daniel Kraft
	Comment 109 • 19 years ago

What I'm curious about is that "counter-reset" can't be specified in the
:before-pseudo-element (as far as I tested). Look for example at the sample CSS
found in the current working draft or CSS 2.1:

<style type="text/css">
H1:before {
    content: "Chapter " counter(chapter) ". ";
    counter-increment: chapter;  /* Add 1 to chapter */
    counter-reset: section;      /* Set section to 0 */
}
H2:before {
    content: counter(chapter) "." counter(section) " ";
    counter-increment: section;
}
</style>

This is supposed to number chapters and sections, but it doesn't increment the
counters. If the counter-reset is moved to h1 instead of h1:before, the sections
are incremented correctly (the chapter counter is never reset, so this is wrong
according to what you wrote). I couldn't figure out why this shouldn't work.
What's the problem?

	Anne (:annevk)
	Comment 110 • 19 years ago

Please read all the comments that say CSS 2.1 is irrelevant here. Please be
patient and wait for the next CSS 2.1 draft which will say exactly what Mozilla
does. (As Mozilla implemented a proposal the CSS WG agreed with.)

	Boris Zbarsky [:bzbarsky]
	Comment 111 • 19 years ago

> can't be specified in the :before-pseudo-element

Sure it can.  It'll be in effect for the scope of the reset: the children of the
:before (there aren't any) and its following siblings (the kids of the <h1>).

But the <h2> is not a child of the <h1>, so the scope of the reset on the
:before doesn't cover it.  As Anne said, one can hope the next draft of CSS2.1
will spell this out better (and fix the examples).

	Daniel Kraft
	Comment 112 • 19 years ago

Thanks! Now I understand the "new system" (and I think it's right and more correct).

	petr.pisar
	Comment 113 • 19 years ago

Can I have a question? I have read very carefully the CSS-2 starndard and this
thread. I'm not sure if it is possible the following numbering:

Let's have this XML code:

<doc>
  <sec>
    <para/>
    <para/>
  </sec>
  <sec>
    <para/>
    <para/>
  </sec>
</doc>

And I want this numbering:

1. section
  (1) paragraph
  (2) paragraph
2. section
  (3) paragraph
  (4) paragraph

I.e. paragraph counter is global in the whole document.

I created simmilar CSS:

* {display: block;}
sec {counter-increment: sec;}
sec:before {content: counter(sec) ".";}
para {counter-increment: para;}
para:before {content: "(" counter(para) ")";}

But the contemporary rendering in firefox is:

1. section
  (1) paragraph
  (2) paragraph
2. section
  (1) paragraph
  (2) paragraph

	James Ross
	Comment 114 • 19 years ago

That seems wrong, according to CSS 2.1 and the current CSS 3 Content draft.

"If 'counter-increment' refers to a counter that is not in the scope (see below)
of any 'counter-reset', the counter is assumed to have been reset to 0 by the
root element."

My reading of that suggests that the second level should have the pretend reset
on <doc>, and thus count 1, 2, 3, 4, but instead it acts like the reset is on
the <sec>.

Putting  doc {counter-reset: para;}  does make it work as desired, whether the
rendering without is correct or not.

	Boris Zbarsky [:bzbarsky]
	Comment 115 • 19 years ago

> That seems wrong, according to CSS 2.1 and the current CSS 3 Content draft.

See comment 108.  The implied reset is not on the root but on the element with
the increment-not-in-reset-scope, so doesn't extend to the paragraphs in
following sections.  Putting an explicit reset on the root element, as James
suggests, is the right thing to do.

	petr.pisar
	Comment 116 • 19 years ago

Thanks for clarification.

I found out some strange undeterministic behaviour in counter incrementation
depending on display property (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2)
Gecko/20050529 Firefox/1.0+).

Let's have this XML code:

<doc>
  <sec>
    <title/>
  </sec>
</doc>

and CSS:

doc {counter-reset: sec;}
sec {
  /*display: block;*/
  counter-increment: sec;
}
title {display: block;}
title:before {content: counter(sec) ".";}

Expexted beahaviour is rendering "1.". But it shows ussualy "2.", sometimes "1."
If I set the display property of sec to "block", it will count right.

	Boris Zbarsky [:bzbarsky]
	Comment 117 • 19 years ago

Filed bug 296083 on comment 116.  In general, please file separate bugs on
followup issues.

	Ng Ming Hong
	Comment 118 • 18 years ago

Excuse me. I know this bug in fixed. But why am I still seeing this bug in Firefox  1.5.0.3? The counter-increment doesn't seem to work... Try out the "simple testcase" above if you don't believe. :-(

	David Baron :dbaron: Assignee
	Comment 119 • 18 years ago

See comment 108.

	Ng Ming Hong
	Comment 120 • 18 years ago

Thanks for the comment. So be sure to put counter-increment on the real element, not in the pseudo-element (:before in this case)...

Interestingly Opera 8.54 supports counter-increment in pseudo-elements as well. Maybe it is a bug?

Anyway, I'll stop here as I don't mean to add meaningless comment in this bug.

	Anne (:annevk)
	Comment 121 • 18 years ago

Opera implemented the old model (although not entirely). Firefox its implementation is based on the current specification (largely based on feedback from David Baron).

	David A. Wheeler
	Comment 122 • 18 years ago

See also Bug 348278 - which is a regression for counters, even when correctly placed.

	David Baron :dbaron: Assignee
	Comment 123 • 17 years ago

My tests are now checked in to the trunk in layout/reftests/counters/, thanks to reftest conversion work by Rob Campbell.