[Webkit-unassigned] [Bug 26084] Multiple missing images in webkit-mask-image prevent rendering
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 1 00:31:53 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=26084
darin at apple.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #30785|review? |review-
Flag| |
------- Comment #2 from darin at apple.com 2009-06-01 00:31 PDT -------
(From update of attachment 30785)
Looks good! Thanks for taking this on.
This patch has tabs in it, so we won't be able to land it as-is. Please
re-submit it with spaces instead of tabs for indentation.
Formatting of the comment is also incorrect. There should be spaces after the
"//" and the comment should use sentence style.
> + const FillLayer* fillLayer = style()->maskLayers()->next();
> + while (fillLayer) {
> + if (fillLayer->hasImage() && fillLayer->image()->canRender(style()->effectiveZoom())) {
> + pushTransparencyLayer = true;
> + break;
> + }
> + fillLayer = fillLayer->next();
> + }
The above loop would read better as a for loop, I think.
The patch needs to include layout test results as well as the new test; they
can be generated with run-webkit-tests. I'm concerned that the results of this
layout test won't indicate success or failure. This may need to be a manual
test instead, or maybe the pixel result is the only way we can tell the test
failed.
review- because of the tabs, formatting, and layout test concerns
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list