Bug 33979

Created attachment 47160 [details] Patch

Comment on attachment 47160 [details] Patch Change looks fine. Resorting to nit-picking. > + <rdar://problem/7378662> > + > + > + > + * http/tests/misc/resources/slow-png-load.pl: Added. Too many newlines. > \ No newline at end of file Too few newlines. > + * rendering/RenderBox.cpp: > + (WebCore::RenderBox::paintMaskImages): > + * rendering/style/FillLayer.cpp: > + (WebCore::FillLayer::areImagesLoaded): > + * rendering/style/FillLayer.h: Explain what you did there? > + bool areImagesLoaded() const; I prefer imagesAreLoaded() or hasLoaded[All]Images().

This test is puzzling me. It fails on Chromium and Qt - and I suspect it really fails on gtk as you can get the same rendertree results regardless of whether you really pass or not ('pass' being defined as not displaying the blue background of the boxes). The problem, as I see it, is that the blue background always gets rendered before the mask in RenderBlock::paintObject() so it will always get painted. Whether the images are loaded or not in RenderBox::paintMaskImages() seems immaterial, since there is nothing there that will paint over the already-rendered background. The other thing I can't get my head around is that isLoaded() is always true for a StyleImage so the code added with this patch will always think the images have been loaded. Maybe something has changed since this patch and these are regressions of some sort. More likely I'm not understanding the code properly. Simon, could you take another look at it? Does it still pass manually on mac?

The pixel test is now failing, so this regressed at some point.

Comment on attachment 47160 [details] Patch Clearing flag on original patch since this bug has been re-broken and needs a new fix.