Words with inline elements should only have one capital letter when text-transform: capitalize is applied. this is in the w3c test, and is mentioned on diveintomark. test case: <html> <head> <style type="text/css"> .cap {text-transform: capitalize;} </style> </head> <body> <P>Words with inline elements inside them should only capitalize the first letter of the word. Therefore, the last word in this sentence should have one, and only one, capital <SPAN CLASS=cap>le<SPAN>tt</SPAN>er</SPAN>. </P> </body> </html>
See my testcase attached to other capitalise bug (#3406) for this, specifically the words 'earthquake' 'earthworm' and 'cheeseburger' in the english and 3eme in the french.
Created attachment 5328 [details] test case as attachement
Created attachment 6458 [details] Proposed patch While this doesn't use UBreakIterator, it does fix the problems we have displaying the CSS1 test suite and should make all capitalisation obey the same rules instead of inlines being able to break rendering (essentially we capitalise the first letter after a space and nothing else). Also added Copyright to StringImpl because I wrote the last fix for capitalisation in here, otherwise it's hardly worth it :) To be attached in a second after this, updated LayoutTests/css2.1/support/css1test545.png for the new capitalisation behaviour to draw the green box correctly.
Created attachment 6459 [details] Updated LayoutTests/css2.1/support/css1test545.png Update to go with patch for background image from test case css2.1/t1605-c545-txttrans-00-b-ag
Comment on attachment 6458 [details] Proposed patch + for (o = previousRenderer(); o && o->isInline() && !o->isText(); o = o->previousRenderer()) This will include images that occur between words, e.g., foo<img>goo I'm not sure that's right. You might want to see what other browsers do. + ; + if (o && o->isText()) { + DOMStringImpl* prevStr = static_cast<RenderText*>(o)->string(); + QChar c = (*prevStr)[prevStr->length() - 1]; + if (!c.isSpace()) + runOnString = true; + } Not sure about this from an RTL perspective. Maybe Mitz could comment, but that seems dangerous to me.
Created attachment 6471 [details] Proposed patch 2 Fix the img case hyatt mentioned. Works correctly with RTL text as discussed with mitzpettel. Added a new LayoutTest to check for img case and RTL case. Will add the greenbox.png to this bug. Will require updated pixel test image for updated tests as well.
Created attachment 6472 [details] greenbox.png for testcase Required for added test case in patch 2, located in LayoutTests/css1/text_properties/support/greenbox.png
Comment on attachment 6471 [details] Proposed patch 2 I think it's OK to land this now. I don't like the name "runOnString" for a boolean that means "has a previous character is not a space", but those are nitpicks and this seems fine. Dave had told me this is a difficult area to get right, but on his previous review he only mentioned the two issues that were investigated and fixed so I'm going to assume all is well. r=me
To Andrew: Does this handle non?breaking spaces, zero?width space, thin spaces, em spaces etc. too? Does it NOT capitalise after zero width joiner, zero width non?joiner?
Nick: This follows the same rules as capitalisation within a single span. From my tests this means that the following "unusual" characters are considered to break a work: Thin Space Em-space En-space Not breaking on: non-breaking space (not sure if we should or not on this one) zero-width non-joiner zero-width joiner The best way to resolve these is probably to use UBreakIterator, or to decide if they should be considered spaces for the isSpace() call.
Sorry about my previous comment, i didn't realise bugzilla couldn't handle unicode. (I have remapped U+2010 HYPHEN to the key between zero and equals.) Word breaking isn't the same thing though, so you shouldn't be using UBreakIterator really. For example hyphenated words like Semi?circle (that's "Semi-circle" using a HYPHEN-MINUS) or Mesopotamia (Mesopotamia with a SOFT HYPHEN after the second syllable) can break after the hyphen but should not capitalise. All of the spaces from U+2002 to U+200B should be followed by a capital. As for the non?breaking space, which as best I can tell is mostly used in cases such as "20 kg" and "Rameses II" should probably not be capitalised due to usage, where for example "100 ?" ? "100 ?".
(In reply to comment #11) > where for example "100 ?" ? "100 ?". Grr. That was supposed to say "100 microns" does not equal "100 capital Mu".
(In reply to comment #11) > Word breaking isn't the same thing though, so you shouldn't be using > UBreakIterator really. That's the reason to not use UBreakIterator's "word boundary analysis". But according to the ICU documentation "title boundary analysis locates all positions, typically starts of words, that should be set to title case when title casing the text." It seems that *is* what we want to use. Anyway, since this bug is fixed, lets file new bug reports about further enhancements and cases that are behaving incorrectly instead of putting more comments here where no one is going to read them.
(In reply to comment #3) > To be attached in a second after this, updated > LayoutTests/css2.1/support/css1test545.png for the new capitalisation behaviour > to draw the green box correctly. I wanted to land this patch, but I don't understand why we have to modify the CSS 1 test suite itself. That PNG came with the test suite -- why do we want to change it?
I committed this fix.