[webkit-reviews] review canceled: [Bug 130457] Improper styling of pseudo element first-letter : [Attachment 227193] Patch proposal

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has canceled Mario Sanchez Prada
<mario at webkit.org>'s request for review:
Bug 130457: Improper styling of pseudo element first-letter
https://bugs.webkit.org/show_bug.cgi?id=130457

Attachment 227193: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=227193&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
After investigating this issue a bit more, it seems to me that the difference
in the results comes from the fact that the backported code came with a
problem, inherited from Blink r159205:

+  if (descendant->isText()) {
+     // FIXME: If there is leading punctuation in a different RenderText than
+     // the first letter, we'll not apply the correct style to it.
+     length = firstLetterLength(toRenderText(*descendant).originalText());
+     if (length)
+	  break;

For instance, in editing/selection/extend-by-word-002.html, for the following
code (where we have defined "ul.menu li:first-letter {
font-size:20px;line-height:10px; }"):

  <li>&middot; <a href="detail.asp?cat=13">Soups & Salads</a></li>

According to the spec, the UA does not need to create a first-letter
pseudoelement in this case, which is what Opera and Firefox do. However, the
present patch applies the first-letter style to the "A" after the dot, which is
wrong. Even worse, I've tried the following code:

  <li>&middot; Starter <a href="detail.asp?cat=13">Soups & Salads</a></li>

And I get the list element rendered applying the first letter style over the
"S" of "Soups", instead of doing nothing (as there's a space after the dot. If
however, I remove the space between the dot and "Starter":

  <li>&middot;Starter <a href="detail.asp?cat=13">Soups & Salads</a></li>

I get consistent rendering across all browsers, applying the style over the "S"
of "Starter". It's a pity I didn't detect this first in Blink, but the way how
extend-by-words-002.html is tested there didn't catch this issue

So, I think I'll get back to blink, try to fix these issues there first and
then will come back here. Now removing flags.