| 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 Rendering | Assignee: | 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
mitz
2012-03-21 10:39:56 PDT
This is a regression. It doesn’t happen in Safari 5.1.4. Added this test to the Mac skipped list in <http://trac.webkit.org/r111571>. This regression occurred between r110065 and r110080. It was most likely caused by <http://trac.webkit.org/r110072>, the fix for bug 75568. Julien, can you take a look at this? (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. Thanks!! Created attachment 133687 [details]
Proposed fix. Unfortunately no new test.
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. 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. Created attachment 133826 [details]
Patch for landing
Comment on attachment 133826 [details] Patch for landing Clearing flags on attachment: 133826 Committed r112114: <http://trac.webkit.org/changeset/112114> All reviewed patches have been landed. Closing bug. |