Bug 22938

Summary: [Transforms] WebView background is not painted correctly if the document has a transform
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: hyatt
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase
none
Patch, testcase, changelog
darin: review-
Fixed patch - restore the !view() check darin: review+

Simon Fraser (smfr)
Reported 2008-12-19 13:09:19 PST
If the document has a transform, via something like: html { -webkit-transform: translateY(100px) rotate(20deg); } then the exposed area of the viewport that is not covered by the document is unpainted, and full of pixel crap.
Attachments
Testcase (1.10 KB, text/html)
2008-12-19 13:10 PST, Simon Fraser (smfr) 		no flags
Details
Patch, testcase, changelog (5.10 KB, patch)
2008-12-19 13:49 PST, Simon Fraser (smfr) 		darin: review-
DetailsFormatted DiffDiff
Fixed patch - restore the !view() check (5.09 KB, patch)
2008-12-19 14:22 PST, Simon Fraser (smfr) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2008-12-19 13:10:08 PST
Created attachment 26152 [details] Testcase
Simon Fraser (smfr)
Comment 2 2008-12-19 13:49:48 PST
Created attachment 26156 [details] Patch, testcase, changelog
Darin Adler
Comment 3 2008-12-19 14:03:34 PST
Comment on attachment 26156 [details] Patch, testcase, changelog Where did the null check of view() go? Removing that could lead to crashes. If it was redundant, then it's OK to remove it, but the change log should explain that.
Simon Fraser (smfr)
Comment 4 2008-12-19 14:16:26 PST
Oops that was a mistake. New patch coming.
Simon Fraser (smfr)
Comment 5 2008-12-19 14:22:02 PST
Created attachment 26158 [details] Fixed patch - restore the !view() check
Darin Adler
Comment 6 2008-12-19 14:31:48 PST
Comment on attachment 26158 [details] Fixed patch - restore the !view() check > + // If painting will entirely fill the view, no need to fill the background Comments in sentence style should end in a period. > + if (elt || > + (firstChild() && (firstChild()->style()->visibility() == VISIBLE && !firstChild()->style()->hasTransform())) || > + !view()) > return; You have an extra set of parentheses in there that I don't think add clarity. You don't need the ones after "firstChild() &&". Our typical format for long if statements like this is to start lines with operators, and also (maybe not a good format) to indent an extra level so the expression doesn't line up with the return statement. if (elt || (firstChild() && firstChild()->style()->visibility() == VISIBLE && !firstChild()->style()->hasTransform()) || !view()) return; But I think the best way to clean this up is with a helper function. static inline bool isVisibleWithNoTransform(RenderObject* object) { return object && object->style()->visibility() == VISIBLE && ! object >style()->hasTransform(); } [...] if (elt || isVisibleWithNoTransform(firstChild()) || !view()) return; [...] I think it's worth it to keep this code readable. > - // This code typically only executes if the root element's visibility has been set to hidden. > + // This code typically only executes if the root element's visibility has been set to hidden, > + // or there is a transform on the <html> Comments in sentence style should end in a period. r=me -- please consider my refinement suggestions.
Simon Fraser (smfr)
Comment 7 2008-12-19 14:44:27 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/transforms/transformed-document-element.html A LayoutTests/platform/mac/fast/transforms/transformed-document-element-expected.checksum A LayoutTests/platform/mac/fast/transforms/transformed-document-element-expected.png A LayoutTests/platform/mac/fast/transforms/transformed-document-element-expected.txt M WebCore/ChangeLog M WebCore/rendering/RenderView.cpp Committed r39413
Note You need to log in before you can comment on or make changes to this bug.