[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