WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
22938
[Transforms] WebView background is not painted correctly if the document has a transform
https://bugs.webkit.org/show_bug.cgi?id=22938
Summary
[Transforms] WebView background is not painted correctly if the document has ...
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-
Details
Formatted Diff
Diff
Fixed patch - restore the !view() check
(5.09 KB, patch)
2008-12-19 14:22 PST
,
Simon Fraser (smfr)
darin
: review+
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug