I talked with Xidorn. Supporting @counter-style would be a lot of work, and Firefox is the only UA to implement it right now so it's a pretty low priority for us at the moment. The machinery initially seems a bit hard to scoot out of the way, but we figured out a strategy. Patches to follow.

Comment on attachment 8748887 [details] [diff] [review] Part 2 - Teach CounterStyles their name and remove the string member from the style structs. v1 Review of attachment 8748887 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/CounterStyleManager.h @@ +50,5 @@ > // Custom styles are certainly dependent. In addition, some builtin > // styles are dependent for fallback. > bool IsDependentStyle() const; > > + virtual void GetStyleName(nsSubstring& aResult) = 0; I think GetName should be enough here, but I can live with GetStyleName as well. ::: layout/style/nsStyleStruct.cpp @@ -737,5 @@ > if (EqualImages(mListStyleImage, aOther.mListStyleImage) && > mCounterStyle == aOther.mCounterStyle) { > if (mImageRegion.IsEqualInterior(aOther.mImageRegion)) { > - if (mListStyleType != aOther.mListStyleType) > - return nsChangeHint_NeutralChange; This change actually indicates this patch does change the behavior in some cases. Specifically, this changes the following two cases: * if list-style-type is a non-existing counter style, the computed value becomes "decimal" rather than the specified value; * if list-style-type refers to a builtin style, the computed value is always lowercased rather than keeps the original case. I guess these changes are fine, probably even somehow perferable. I'll raise this with CSSWG.

Comment on attachment 8748888 [details] [diff] [review] Part 3 - Add hooks for Servo to manipulate list-style-type. v1 Review of attachment 8748888 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/CounterStyleManager.cpp @@ +1001,1 @@ > NS_DECL_OWNINGTHREAD If we are going to refcount the pointers off-main-thread, is there still any owning thread here? (Actually I don't see why we need this decl here. It doesn't seem to be used anywhere. I think I just copied it from some other place without understanding the reason.) In addition, this object is allocated from the arena, would there be any issue in terms of thread-safety for that? It seems you are not going to support custom counter style (and probably also complex CJK counter styles) at this point, so this change is actually not needed at present. I guess you can probably just assert somewhere that this would never be created within Servo's style. @@ +1191,5 @@ > // not 'extends'. > CounterStyle* mExtendsRoot; > > + // Parallel restyle may refcount these pointers off-main-thread. > + ThreadSafeAutoRefCnt mRefCnt; Same here.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #6) > Comment on attachment 8748888 [details] [diff] [review] > Part 3 - Add hooks for Servo to manipulate list-style-type. v1 > > Review of attachment 8748888 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/style/CounterStyleManager.cpp > @@ +1001,1 @@ > > NS_DECL_OWNINGTHREAD > > If we are going to refcount the pointers off-main-thread, is there still any > owning thread here? (Actually I don't see why we need this decl here. It > doesn't seem to be used anywhere. I think I just copied it from some other > place without understanding the reason.) There isn't an owning thread, but the the addref/release macros still won't compile without it: http://searchfox.org/mozilla-central/rev/64a1e03ff39d72e0f9c138af0af4fda99606c54d/xpcom/glue/nsISupportsImpl.h#571 > In addition, this object is allocated from the arena, would there be any > issue in terms of thread-safety for that? I guess we'd need to proxy-release on the main thread in the destroy method - that's a good point. > > It seems you are not going to support custom counter style (and probably > also complex CJK counter styles) at this point, so this change is actually > not needed at present. I guess you can probably just assert somewhere that > this would never be created within Servo's style. Fair enough.

Comment on attachment 8749404 [details] [diff] [review] Part 3 - Add hooks for Servo to manipulate list-style-type. v2 Review of attachment 8749404 [details] [diff] [review]: ----------------------------------------------------------------- OK, thanks.

Although I r+ed these patches, but it seems neither you nor I am peer of style system, so I'm not completely sure whether I'm qualified to do this. Probably need heycam to double check.

Patches look OK to me.

ni? myself for emailing CSSWG mentioned in comment 5.

https://hg.mozilla.org/integration/mozilla-inbound/rev/382ae5a47588 https://hg.mozilla.org/integration/mozilla-inbound/rev/922ba5fbebbe https://hg.mozilla.org/integration/mozilla-inbound/rev/94fe66b7e3c2

https://hg.mozilla.org/mozilla-central/rev/382ae5a47588 https://hg.mozilla.org/mozilla-central/rev/922ba5fbebbe https://hg.mozilla.org/mozilla-central/rev/94fe66b7e3c2