Bug 33979

Summary: -webkit-mask-box-image draws a box while loading
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: REOPENED ---    
Severity: Normal CC: krit, robert, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 95389, 34004    
Attachments:
Description Flags
Patch none

Description Simon Fraser (smfr) 2010-01-21 17:00:45 PST
-webkit-mask-box-image draws a box while loading
Comment 1 Simon Fraser (smfr) 2010-01-21 17:06:27 PST
Created attachment 47160 [details]
Patch
Comment 2 mitz 2010-01-21 17:16:55 PST
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().
Comment 3 Simon Fraser (smfr) 2010-01-21 17:28:30 PST
http://trac.webkit.org/changeset/53663
Comment 4 Robert Hogan 2011-05-29 10:52:10 PDT
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?
Comment 5 Simon Fraser (smfr) 2011-05-29 11:08:06 PDT
The pixel test is now failing, so this regressed at some point.
Comment 6 Darin Adler 2011-06-18 12:19:43 PDT
Comment on attachment 47160 [details]
Patch

Clearing flag on original patch since this bug has been re-broken and needs a new fix.