[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 03:30:40 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=26084
------- Comment #3 from mihnea at adobe.com 2009-06-01 03:30 PDT -------
(In reply to comment #2)
> (From update of attachment 30785 [review])
> 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
>
Thank you for your time spent reviewing this. I will take time to rework the
patch and address your comments :)
--
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