Changeset 112114 in webkit

Timestamp:

Mar 26, 2012 9:52:01 AM (12 years ago)

Author:

jchaffraix@webkit.org

Message:

REGRESSION (r110065-r110080): fast/forms/placeholder-set-attribute.html is failing intermittently because WebKit fails to repaint after setting the placeholder attribute
​https://bugs.webkit.org/show_bug.cgi?id=81802

Reviewed by Dan Bernstein.

Source/WebCore:

Covered by fast/forms/placeholder-set-attribute.html which should be less flaky.

Unfortunately no new test case as this bug requires a very specific set of conditions that I couldn't reproduce deterministically.

This is a regression from r110072: RenderTextControlSingleLine would rely on the placeholder's RenderLayer to properly repaint during
the first layout as the placeholder has overflow: hidden set. r110072 removed the layer in this case and thus we miss a repaint.

• rendering/RenderTextControlSingleLine.cpp:

(WebCore::RenderTextControlSingleLine::layout):
For our first layout, we need to make sure our placeholder is painted. layoutBlockChild has a very similar logic to force repaint
on a first layout. We don't hit this logic as the placeholder is explicitly skipped in RenderTextControl::layoutSpecialExcludedChild.

LayoutTests:

• platform/mac/Skipped: Unskip the test on Mac. The other platforms are either

skipping it for other reasons or did not have a specific entry for the flakiness.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/Skipped (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderTextControlSingleLine.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-26  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        REGRESSION (r110065-r110080): fast/forms/placeholder-set-attribute.html is failing intermittently because WebKit fails to repaint after setting the placeholder attribute
+        https://bugs.webkit.org/show_bug.cgi?id=81802
+
+        Reviewed by Dan Bernstein.
+
+        * platform/mac/Skipped: Unskip the test on Mac. The other platforms are either
+        skipping it for other reasons or did not have a specific entry for the flakiness.
+
 2012-03-26  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/LayoutTests/platform/mac/Skipped

@@ -605 +605 @@
 fast/inline/continuation-outlines-with-layers.html
 
-# https://bugs.webkit.org/show_bug.cgi?id=81802 rdar://problem/11091412
-# fast/forms/placeholder-set-attribute.html is failing intermittently because WebKit fails to repaint after setting the placeholder attribute
-fast/forms/placeholder-set-attribute.html
-
 # Not possible to set UA string for media requests with all Mac media engines.
 http/tests/media/video-useragent.html

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-26  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        REGRESSION (r110065-r110080): fast/forms/placeholder-set-attribute.html is failing intermittently because WebKit fails to repaint after setting the placeholder attribute
+        https://bugs.webkit.org/show_bug.cgi?id=81802
+
+        Reviewed by Dan Bernstein.
+
+        Covered by fast/forms/placeholder-set-attribute.html which should be less flaky.
+
+        Unfortunately no new test case as this bug requires a very specific set of conditions that I couldn't reproduce deterministically.
+
+        This is a regression from r110072: RenderTextControlSingleLine would rely on the placeholder's RenderLayer to properly repaint during
+        the first layout as the placeholder has overflow: hidden set. r110072 removed the layer in this case and thus we miss a repaint.
+
+        * rendering/RenderTextControlSingleLine.cpp:
+        (WebCore::RenderTextControlSingleLine::layout):
+        For our first layout, we need to make sure our placeholder is painted. layoutBlockChild has a very similar logic to force repaint
+        on a first layout. We don't hit this logic as the placeholder is explicitly skipped in RenderTextControl::layoutSpecialExcludedChild.
+
 2012-03-26  Julien Chaffraix  <jchaffraix@webkit.org>
 

trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp

@@ -280 +280 @@
         placeholderBox->style()->setWidth(Length(innerTextRenderer->width() - placeholderBox->borderAndPaddingWidth(), Fixed));
         placeholderBox->style()->setHeight(Length(innerTextRenderer->height() - placeholderBox->borderAndPaddingHeight(), Fixed));
+        bool placeholderBoxHadLayout = placeholderBox->everHadLayout();
         placeholderBox->layoutIfNeeded();
         LayoutPoint textOffset = innerTextRenderer->location();
@@ -287 +288 @@
             textOffset += toLayoutSize(containerRenderer->location());
         placeholderBox->setLocation(textOffset);
+
+        if (!placeholderBoxHadLayout && placeholderBox->checkForRepaintDuringLayout()) {
+            // This assumes a shadow tree without floats. If floats are added, the
+            // logic should be shared with RenderBlock::layoutBlockChild.
+            placeholderBox->repaint();
+        }
     }
 }