Bug 32138

Created attachment 44753 [details] Patch Work in progress. Will try to put together a layout test/pixel test sometime tomorrow for this.

Created attachment 45250 [details] Patch with test case Added missing Unicode code points U+03A4 (Tau), U+03A5 (Upsilon), U+03A6 (Phi), U+03A7 (Chi), and U+03A8 (Psi). Added DRT layout-test/pixel-test results for the Mac and Win builds.

style-queue ran check-webkit-style on attachment 45250 [details] without any errors.

CC'ing Ian Hickson, Darin Adler, Dave Hyatt, Beth Dakin, Dan Bernstein, and Alexey Proskuryakov as they either have knowledge of CSS/rendering and/or may be interested in following this bug since it relates to bug #3232.

I would prefer to have a non-pixel test if possible. I'd also prefer to have approach that makes it easy to add new tests for new list types. Ideally it would single test for many list types rather than a separate test for each one. Maybe a test for each category from CSS 3. The CSS 2.1 specification lists the following 13 list-style types: disc | circle | square | decimal | decimal-leading-zero | lower-roman | upper-roman | lower-greek | lower-latin | upper-latin | armenian | georgian | lower-alpha | upper-alpha The 2002-10-07 CSS 3 Lists module working draft includes the following 100 list-style types: box | check | circle | diamond | disc | hyphen | square | armenian | cjk-ideographic | ethiopic-numeric | georgian | hebrew | japanese-formal | japanese-informal | lower-armenian | lower-roman | simp-chinese-formal | simp-chinese-informal | syriac | tamil | trad-chinese-formal | trad-chinese-informal | upper-armenian | upper-roman | arabic-indic | binary | bengali | cambodian | decimal | decimal-leading-zero | devanagari | gujarati | gurmukhi | kannada | khmer | lao | lower-hexadecimal | malayalam | mongolian | myanmar | octal | oriya | persian | telugu | tibetan | thai | upper-hexadecimal | urdu | afar | amharic | amharic-abegede | cjk-earthly-branch | cjk-heavenly-stem | ethiopic | ethiopic-abegede | ethiopic-abegede-am-et | ethiopic-abegede-gez | ethiopic-abegede-ti-er | ethiopic-abegede-ti-et | ethiopic-halehame-aa-er | ethiopic-halehame-aa-et | ethiopic-halehame-am-et | ethiopic-halehame-gez | ethiopic-halehame-om-et | ethiopic-halehame-sid-et | ethiopic-halehame-so-et | ethiopic-halehame-ti-er | ethiopic-halehame-ti-et | ethiopic-halehame-tig | hangul | hangul-consonant | hiragana | hiragana-iroha | katakana | katakana-iroha | lower-alpha | lower-greek | lower-norwegian | lower-latin | oromo | sidama | somali | tigre | tigrinya-er | tigrinya-er-abegede | tigrinya-et | tigrinya-et-abegede | upper-alpha | upper-greek | upper-norwegian | upper-latin | asterisks | footnotes | circled-decimal | circled-lower-latin | circled-upper-latin | dotted-decimal | double-circled-decimal | filled-circled-decimal | parenthesised-decimal | parenthesised-lower-latin We support 20 types. To me, upper-greek doesn't obviously stand out as the most important of the 80 that are in the CSS 3 draft but not yet in WebKit. I'm slightly curious what inspired you to tackle upper-greek in particular. Before we add lots more values to EListStyleType, we probably want to change the names of the value constants so they aren't in ALL_CAPITALS because that’s not really the preferred style.

A possible idea for how to test new list types is to use the accessibility API. Or we could add some API just for DumpRenderTree so we could make dumpAsText tests. It would be nice not to have to use render tree dumps for everything. On the other hand, we do dump out the character for each RenderListMarker in RenderTreeAsText, so we don't really need to look at the pixel test results these days.

Comment on attachment 45250 [details] Patch with test case I do not understand why the mac and win results are different for the test!

(In reply to comment #5) > I would prefer to have a non-pixel test if possible. I'd also prefer to have > approach that makes it easy to add new tests for new list types. Ideally it > would single test for many list types rather than a separate test for each one. > Maybe a test for each category from CSS 3. Maybe we can just use render-tree results without a pixel test, since DRT's dumpAsText() doesn't write out the list markers. Sure, we can make a single DRT test for ll the CSS list-style-types, similar to LayoutTests/fast/lists/w3-list-styles.html <http://trac.webkit.org/browser/trunk/LayoutTests/fast/lists/w3-list-styles.html>. I'll take a look at creating such a test file. > The CSS 2.1 specification lists the following 13 list-style types: disc | > [...] > upper-alpha > > The 2002-10-07 CSS 3 Lists module working draft includes the following 100 > list-style types: box | check | circle | diamond | disc | hyphen | square | > [....] > parenthesised-decimal | parenthesised-lower-latin > > We support 20 types. > > To me, upper-greek doesn't obviously stand out as the most important of the 80 > that are in the CSS 3 draft but not yet in WebKit. I'm slightly curious what > inspired you to tackle upper-greek in particular. I'm a fan of the greek alphabet ;-) I wanted to first address this bug. Then file a separate bug to add the rest. My only concern was how many of these would be finalized in the standard. I think we should just add them all. We can always remove them, if necessary. Do you have any thoughts on this matter? > Before we add lots more values to EListStyleType, we probably want to change > the names of the value constants so they aren't in ALL_CAPITALS because that’s > not really the preferred style. I filed bug #32799 to address this issue.

It is unclear to me how to use the accessibility API to do this, at this time. I try to take a look at adding support (some optional function call) for dumping the list markers in DRT as part of the dumpAsText. For this bug, I think it would be sufficient to just use a render tree dump without a pixel test. (In reply to comment #6) > A possible idea for how to test new list types is to use the accessibility API. > Or we could add some API just for DumpRenderTree so we could make dumpAsText > tests. It would be nice not to have to use render tree dumps for everything. On > the other hand, we do dump out the character for each RenderListMarker in > RenderTreeAsText, so we don't really need to look at the pixel test results > these days.

(In reply to comment #7) > (From update of attachment 45250 [details]) > I do not understand why the mac and win results are different for the test! Most likely because I don't have the Apple-certified Windows test fonts. Hence, there are render/pixel differences. I'll remove the Windows render/pixel tests and hopefully the Win test bot will pass. If not, I will follow up with some of the Windows guys (i.e. Brian Weinstein, Adam Roben) to fix it. I will try to contact them in advance as well.

You can get the windows test fonts out of the Safari bundle I think. I could have sworn that they were available from one of Qt's servers at some point as well.

Instead of landing this patch, I landed the patch for bug #33089, which implements all of the CSS3 alphabetic list-style-types (including upper-greek).