[Webkit-unassigned] [Bug 26084] Multiple missing images in webkit-mask-image prevent rendering

[bugzilla-daemon at webkit.org]

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.