Bug 81802

Summary: REGRESSION (r110065-r110080): fast/forms/placeholder-set-attribute.html is failing intermittently because WebKit fails to repaint after setting the placeholder attribute
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, jchaffraix, jonlee, tkent, webkit-bug-importer, webkit.review.bot
Priority: P1 Keywords: InRadar, LayoutTestFailure, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://trac.webkit.org/export/111546/trunk/LayoutTests/fast/forms/placeholder-set-attribute.html
Attachments:
Description Flags
Proposed fix. Unfortunately no new test.
none
Patch for landing none

mitz
Reported 2012-03-21 10:39:56 PDT
To reproduce, open fast/forms/placeholder-set-attribute.html in Safari. Reload a few times. Occasionally, the placeholder is missing. Forcing a repaint makes it appear in those cases. This is causing intermittent pixel failures.
Attachments
Proposed fix. Unfortunately no new test. (4.55 KB, patch)
2012-03-25 12:36 PDT, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Patch for landing (4.75 KB, patch)
2012-03-26 09:10 PDT, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2012-03-21 10:40:38 PDT
<rdar://problem/11091412>
mitz
Comment 2 2012-03-21 10:42:43 PDT
This is a regression. It doesn’t happen in Safari 5.1.4.
mitz
Comment 3 2012-03-21 10:43:48 PDT
Added this test to the Mac skipped list in <http://trac.webkit.org/r111571>.
mitz
Comment 4 2012-03-21 10:50:44 PDT
This regression occurred between r110065 and r110080. It was most likely caused by <http://trac.webkit.org/r110072>, the fix for bug 75568.
Adele Peterson
Comment 5 2012-03-23 10:41:27 PDT
Julien, can you take a look at this?
Julien Chaffraix
Comment 6 2012-03-23 10:55:56 PDT
(In reply to comment #5) > Julien, can you take a look at this? Sure, sorry I am not as responsive as I would like on those regressions. I should have some time next week to investigate this bug.
Adele Peterson
Comment 7 2012-03-23 11:41:22 PDT
Thanks!!
Julien Chaffraix
Comment 8 2012-03-25 12:36:14 PDT
Created attachment 133687 [details] Proposed fix. Unfortunately no new test.
mitz
Comment 9 2012-03-25 13:52:27 PDT
Comment on attachment 133687 [details] Proposed fix. Unfortunately no new test. View in context: https://bugs.webkit.org/attachment.cgi?id=133687&action=review r=me if you correct the style > Source/WebCore/ChangeLog:11 > + 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 NOBODY (OOPS!). > + > + 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. > + Can you explain how the change (presumably r110072) caused this regression? > Source/WebCore/rendering/RenderTextControlSingleLine.cpp:297 > + 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(); This multi-line clause needs to be in braces.
Julien Chaffraix
Comment 10 2012-03-25 14:20:27 PDT
Comment on attachment 133687 [details] Proposed fix. Unfortunately no new test. View in context: https://bugs.webkit.org/attachment.cgi?id=133687&action=review >> Source/WebCore/ChangeLog:11 >> + > > Can you explain how the change (presumably r110072) caused this regression? Sure, currently repainting goes through different means and one of them is RenderLayer. In this case, the placeholder has overflow: hidden set and was implicitly relying on the layer to properly repaint the first time. r110072 removed the layer, removing also the first repaint. I will add this information in the ChangeLog.
Julien Chaffraix
Comment 11 2012-03-26 09:10:19 PDT
Created attachment 133826 [details] Patch for landing
WebKit Review Bot
Comment 12 2012-03-26 09:52:17 PDT
Comment on attachment 133826 [details] Patch for landing Clearing flags on attachment: 133826 Committed r112114: <http://trac.webkit.org/changeset/112114>
WebKit Review Bot
Comment 13 2012-03-26 09:52:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.