Bug 32138 - Upper-greek list-style not supported
Summary: Upper-greek list-style not supported
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL: http://www.w3.org/TR/css3-lists/#alph...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-03 16:04 PST by Adam M-W
Modified: 2010-01-18 19:14 PST (History)
9 users (show)

See Also:


Attachments
Patch (4.56 KB, patch)
2009-12-13 05:28 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch with test case (147.77 KB, patch)
2009-12-19 15:42 PST, Daniel Bates
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam M-W 2009-12-03 16:04:29 PST
Lower-greek is supported but upper-greek is not. It is defined as "Α Β Γ Δ Ε Ζ Η Θ Ι Κ Λ Μ Ν Ξ Ο Π Ρ Σ Τ Υ Φ Χ Ψ Ω" in the CSS3 Draft.
Unicode values are U+0391, U+0392, U+0393, U+0394, U+0395, U+0396, U+0397, U+0398, U+0399, U+039A, U+039B, U+039C, U+039D, U+039E, U+039F, U+03A0, U+03A1, U+03A3, U+03A9.

Code should be similar to lower-greek and can be replicated to implement support.
Comment 1 Daniel Bates 2009-12-13 05:28:28 PST
Created attachment 44753 [details]
Patch

Work in progress.

Will try to put together a layout test/pixel test sometime tomorrow for this.
Comment 2 Daniel Bates 2009-12-19 15:42:23 PST
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.
Comment 3 WebKit Review Bot 2009-12-19 15:46:09 PST
style-queue ran check-webkit-style on attachment 45250 [details] without any errors.
Comment 4 Daniel Bates 2009-12-19 15:49:44 PST
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.
Comment 5 Darin Adler 2009-12-19 17:53:23 PST
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.
Comment 6 Darin Adler 2009-12-19 18:02:18 PST
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 7 Darin Adler 2009-12-19 18:05:50 PST
Comment on attachment 45250 [details]
Patch with test case

I do not understand why the mac and win results are different for the test!
Comment 8 Daniel Bates 2009-12-20 13:13:57 PST
(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.
Comment 9 Daniel Bates 2009-12-20 13:27:03 PST
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.
Comment 10 Daniel Bates 2009-12-20 13:34:29 PST
(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.
Comment 11 Eric Seidel (no email) 2009-12-28 23:18:38 PST
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.
Comment 12 Daniel Bates 2010-01-18 19:14:07 PST
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).