RESOLVED FIXED Bug 81447
CSS 2.1 failure: content-counter-010.htm fails
https://bugs.webkit.org/show_bug.cgi?id=81447
Summary CSS 2.1 failure: content-counter-010.htm fails
Robert Hogan
Reported 2012-03-17 03:38:09 PDT
Attachments
Patch (7.47 KB, patch)
2012-03-19 09:34 PDT, Robert Hogan
no flags
test for 700x (385 bytes, text/html)
2012-08-31 17:34 PDT, Alexey Proskuryakov
no flags
Comparison with Firefox 15 (8.99 KB, image/png)
2012-09-04 12:23 PDT, Alexey Proskuryakov
no flags
Patch (13.32 KB, patch)
2012-09-09 04:39 PDT, Robert Hogan
ap: review+
ap: commit-queue-
Alexey Proskuryakov
Comment 1 2012-03-18 20:30:35 PDT
Duplicate of bug 48000?
Robert Hogan
Comment 2 2012-03-19 09:34:59 PDT
Robert Hogan
Comment 3 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.
Robert Hogan
Comment 4 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!
Alexey Proskuryakov
Comment 5 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.
Robert Hogan
Comment 6 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
Alexey Proskuryakov
Comment 7 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.
Robert Hogan
Comment 8 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.
Alexey Proskuryakov
Comment 9 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?
Robert Hogan
Comment 10 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.
Alexey Proskuryakov
Comment 11 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.
Robert Hogan
Comment 12 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.
Alexey Proskuryakov
Comment 13 2012-09-06 11:43:45 PDT
Can you provide evidence that Firefox is incorrect here?
Robert Hogan
Comment 14 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!
Tab Atkins
Comment 15 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.)
Alexey Proskuryakov
Comment 16 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.
Alexey Proskuryakov
Comment 17 2012-09-06 16:14:45 PDT
Please do take the necessary steps to get the test fixed.
Tab Atkins
Comment 18 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.
Alexey Proskuryakov
Comment 19 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.
Robert Hogan
Comment 20 2012-09-09 04:39:53 PDT
Alexey Proskuryakov
Comment 21 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.
Robert Hogan
Comment 22 2012-09-10 10:42:34 PDT
Note You need to log in before you can comment on or make changes to this bug.