Bug 81447

Summary: CSS 2.1 failure: content-counter-010.htm fails
Product: WebKit Reporter: Robert Hogan <robert>
Component: CSSAssignee: Robert Hogan <robert>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dbaron, dbates, eric, robert, tabatkins, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://test.csswg.org/suites/css2.1/nightly-unstable/html4/content-counter-010.htm
Bug Depends on:    
Bug Blocks: 47141, 48000    
Attachments:
Description Flags
Patch
none
test for 700x
none
Comparison with Firefox 15
none
Patch ap: review+, ap: commit-queue-

Description Robert Hogan 2012-03-17 03:38:09 PDT
Per http://lists.w3.org/Archives/Public/public-css-testsuite/2010Nov/0064.html, armenian 7000 is currently incorrect.
Comment 1 Alexey Proskuryakov 2012-03-18 20:30:35 PDT
Duplicate of bug 48000?
Comment 2 Robert Hogan 2012-03-19 09:34:59 PDT
Created attachment 132598 [details]
Patch
Comment 3 Robert Hogan 2012-03-19 09:41:31 PDT
Comment on attachment 132598 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132598&action=review

> LayoutTests/ChangeLog:11
> +          Note that <meta http-equiv="Content-Type" content="text/html; charset=utf-8"  was added to the suite test 
> +          to allow it to render correctly from a file:/ url.

Firefox is the same, though Opera is not.
Comment 4 Robert Hogan 2012-03-19 09:42:36 PDT
(In reply to comment #1)
> Duplicate of bug 48000?

There seems to be a lot more wrong with Armenian than just this!
Comment 5 Alexey Proskuryakov 2012-03-19 11:10:49 PDT
Comment on attachment 132598 [details]
Patch

Please make sure that number 7001 and so on are also displayed correctly, not just 7000. I believe that they are still wrong - they are certainly not the same as in Firefox.
Comment 6 Robert Hogan 2012-03-19 14:02:03 PDT
(In reply to comment #5)
> (From update of attachment 132598 [details])
> Please make sure that number 7001 and so on are also displayed correctly, not just 7000. I believe that they are still wrong - they are certainly not the same as in Firefox.

My reading of of the links from http://lists.w3.org/Archives/Public/public-css-testsuite/2010Nov/0064.html, particularly http://dev.w3.org/cvsweb/csswg/css3-lists/Attic/Overview.src.html.diff?r1=1.3&r2=1.7, is that only 7000 needs to be fixed, see the fourth-last and third-last entries in this test:

http://www.w3.org/International/tests/tests-html-css/tests-list-style-type/generate?test=2
Comment 7 Alexey Proskuryakov 2012-03-19 14:18:38 PDT
I think that this needs to be investigated further - it's quite strange to have this test fail in Mozilla given that Mozilla folks are the ones polishing this functionality so hard. CC'ing the test author.
Comment 8 Robert Hogan 2012-08-29 10:57:28 PDT
(In reply to comment #7)
> I think that this needs to be investigated further - it's quite strange to have this test fail in Mozilla given that Mozilla folks are the ones polishing this functionality so hard. CC'ing the test author.

This test is now passing on FF 14.1 and Opera 12.00.

So this patch will bring us into line with the revised spec and both other browsers.
Comment 9 Alexey Proskuryakov 2012-08-31 17:34:33 PDT
Created attachment 161797 [details]
test for 700x

Does this test look the same in Gecko and in WebKit with your patch?
Comment 10 Robert Hogan 2012-09-01 00:50:11 PDT
(In reply to comment #9)
> Created an attachment (id=161797) [details]
> test for 700x
> 
> Does this test look the same in Gecko and in WebKit with your patch?

That's correct.
Comment 11 Alexey Proskuryakov 2012-09-04 12:23:19 PDT
Created attachment 162078 [details]
Comparison with Firefox 15

> > Does this test look the same in Gecko and in WebKit with your patch?
> That's correct.

I applied the patch, and this is not what I'm seeing. Rendering is different between us and Firefox 15, please see the attachment.
Comment 12 Robert Hogan 2012-09-06 11:21:02 PDT
(In reply to comment #11)
> Created an attachment (id=162078) [details]
> Comparison with Firefox 15
> 
> > > Does this test look the same in Gecko and in WebKit with your patch?
> > That's correct.
> 
> I applied the patch, and this is not what I'm seeing. Rendering is different between us and Firefox 15, please see the attachment.

Sorry - this was pure inattention on my part, I thought your question was referring to the test in the patch - I didn't register that you'd added a separate test. And I'd forgotten this all goes back to Firefox still rendering 7001 and above with  Ւ rather than ՈՒ.

So as long as Opera and Firefox continue to fail the last two line in http://www.w3.org/International/tests/tests-html-css/tests-list-style-type/generate?test=2 this ambiguity will remain I guess.
Comment 13 Alexey Proskuryakov 2012-09-06 11:43:45 PDT
Can you provide evidence that Firefox is incorrect here?
Comment 14 Robert Hogan 2012-09-06 14:18:45 PDT
(In reply to comment #13)
> Can you provide evidence that Firefox is incorrect here?

My basis for saying so is Firefox's rendering of the third-last and fourth-last lines in:

http://www.w3.org/International/tests/tests-html-css/tests-list-style-type/generate?test=2


But it looks like that test may be incorrect:

http://lists.w3.org/Archives/Public/public-css-testsuite/2012Sep/0007.html

I guess I'll just update the patch to match FF and Opera so!
Comment 15 Tab Atkins 2012-09-06 15:47:26 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > Can you provide evidence that Firefox is incorrect here?
> 
> My basis for saying so is Firefox's rendering of the third-last and fourth-last lines in:
> 
> http://www.w3.org/International/tests/tests-html-css/tests-list-style-type/generate?test=2
> 
> 
> But it looks like that test may be incorrect:
> 
> http://lists.w3.org/Archives/Public/public-css-testsuite/2012Sep/0007.html
> 
> I guess I'll just update the patch to match FF and Opera so!

As far as I can tell, the correct rendering for 7001 is ՒԱ, not ՈՒԱ.  That's what's currently specified in the Counter Styles draft <http://dev.w3.org/csswg/css-counter-styles/#armenian>, and the change that made this so (a couple of years ago) was based on research by Richard Ishida, talking to an Armenian scholar.

(For background, Ո is always placed before a Ւ letter *in writing*, since the spelling reform last century.  However, this is not usually done when Ւ is used as a number digit.)
Comment 16 Alexey Proskuryakov 2012-09-06 16:13:40 PDT
Comment on attachment 132598 [details]
Patch

I'm not super happy with the reasoning in <http://lists.w3.org/Archives/Public/public-css-testsuite/2012Sep/0007.html>. Why refer to a previous e-mail thread and not something authoritative?

Generally, this makes sense though, so I'd be happy with the new patch as you suggest.
Comment 17 Alexey Proskuryakov 2012-09-06 16:14:45 PDT
Please do take the necessary steps to get the test fixed.
Comment 18 Tab Atkins 2012-09-07 15:31:54 PDT
(In reply to comment #16)
> (From update of attachment 132598 [details])
> I'm not super happy with the reasoning in <http://lists.w3.org/Archives/Public/public-css-testsuite/2012Sep/0007.html>. Why refer to a previous e-mail thread and not something authoritative?

Authoritative sources are hard to find for things like this.  For many of these, my spec is actually the first easily-accessible authoritative document writing these things down in English.

However, Richard Ishida has been an extremely important and respected part of the W3C's internationalization work for many years, so I trust his judgement and his investigations.

> Generally, this makes sense though, so I'd be happy with the new patch as you suggest.

Cool.
Comment 19 Alexey Proskuryakov 2012-09-07 15:52:51 PDT
CLDR data is quite authoritative, as it's very aggressively vetted by local experts, both directly and through reporting bugs to vendors that use ICU. For this specific issue however, it's not clear from cursory reading of LDML spec if data format would have allowed for logic like "7 in 7000 is different than in 7001".

http://unicode.org/repos/cldr/trunk/common/rbnf/root.xml
http://www.unicode.org/reports/tr35/

I don't really think that getting to the very bottom of this is necessary for finishing this patch.
Comment 20 Robert Hogan 2012-09-09 04:39:53 PDT
Created attachment 162989 [details]
Patch
Comment 21 Alexey Proskuryakov 2012-09-09 15:46:12 PDT
Comment on attachment 162989 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162989&action=review

> LayoutTests/fast/css/content-counter-010-expected.html:4
> +  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />

Please remove the trailing '/', it shouldn't be present in HTML.

> LayoutTests/fast/css/content-counter-010.htm:4
> +  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <!-- This line was added to the suite test to allow it to render correctly from a file:/ url. -->

Ditto.

I don't think that this is really worth commenting about.
Comment 22 Robert Hogan 2012-09-10 10:42:34 PDT
Committed r128078: <http://trac.webkit.org/changeset/128078>