Changeset 59247 in webkit

Timestamp:

May 12, 2010 12:03:58 PM (14 years ago)

Author:

hyatt@apple.com

Message:

​https://bugs.webkit.org/show_bug.cgi?id=38891

Reviewed by Darin Adler.

First-letter had a number of bugs that were exposed by my attempt to optimize the setting of styles when updating first-letter.
The code that drills down to find the first-letter child stopped if it hit an element that didn't need layout. This means it could
return random incorrect results (and cause the first-letter object to not be found).

In addition when the first-letter was floated/positioned, the text child was not correctly returned, but the container itself was
returned instead.

Finally, the updating code was leaving the box that wrapped the first letter text with a stale style. The old code happened to work because
it made new styles for the text elements instead of using the enclosing box style. The regression was caused by my change to make the
text children simply share style with their parent (thus making the bug that the parent had the wrong style become more prominent).

No new tests, since there's a timing component to reproducing the issue.

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::updateFirstLetter):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderBlock.cpp (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-05-12  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=38891
+
+        First-letter had a number of bugs that were exposed by my attempt to optimize the setting of styles when updating first-letter.
+        The code that drills down to find the first-letter child stopped if it hit an element that didn't need layout.  This means it could
+        return random incorrect results (and cause the first-letter object to not be found).
+
+        In addition when the first-letter was floated/positioned, the text child was not correctly returned, but the container itself was
+        returned instead.
+
+        Finally, the updating code was leaving the box that wrapped the first letter text with a stale style.  The old code happened to work because
+        it made new styles for the text elements instead of using the enclosing box style.  The regression was caused by my change to make the
+        text children simply share style with their parent (thus making the bug that the parent had the wrong style become more prominent).
+
+        No new tests, since there's a timing component to reproducing the issue.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::updateFirstLetter):
+
 2010-05-12  Enrica Casucci  <enrica@apple.com>
 

trunk/WebCore/rendering/RenderBlock.cpp

@@ -4650 +4650 @@
     // Drill into inlines looking for our first text child.
     RenderObject* currChild = firstLetterBlock->firstChild();
-    while (currChild && currChild->needsLayout() && ((!currChild->isReplaced() && !currChild->isRenderButton() && !currChild->isMenuList()) || currChild->isFloatingOrPositioned()) && !currChild->isText()) {
+    while (currChild && ((!currChild->isReplaced() && !currChild->isRenderButton() && !currChild->isMenuList()) || currChild->isFloatingOrPositioned()) && !currChild->isText()) {
         if (currChild->isFloatingOrPositioned()) {
-            if (currChild->style()->styleType() == FIRST_LETTER)
+            if (currChild->style()->styleType() == FIRST_LETTER) {
+                currChild = currChild->firstChild();
                 break;
+            } 
             currChild = currChild->nextSibling();
         } else
@@ -4670 +4672 @@
     // If the child already has style, then it has already been created, so we just want
     // to update it.
-    if (currChild->style()->styleType() == FIRST_LETTER) {
+    if (firstLetterContainer->style()->styleType() == FIRST_LETTER) {
         RenderStyle* pseudo = firstLetterBlock->getCachedPseudoStyle(FIRST_LETTER,
-                                                                     firstLetterContainer->firstLineStyle());
-        currChild->setStyle(pseudo);
-        for (RenderObject* genChild = currChild->firstChild(); genChild; genChild = genChild->nextSibling()) {
+                                                                     firstLetterContainer->parent()->firstLineStyle());
+        firstLetterContainer->setStyle(pseudo);
+        for (RenderObject* genChild = firstLetterContainer->firstChild(); genChild; genChild = genChild->nextSibling()) {
             if (genChild->isText()) 
                 genChild->setStyle(pseudo);