Closed Bug 453566 Opened 16 years ago Closed 16 years ago

HTML element with rgba(0,0,0,0) background produces garbage on screen

Categories

(Core :: Layout: Block and Inline, defect)

Product:
Core
Component:
Layout: Block and Inline
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.1b1

People

(Reporter: zwol, Assigned: zwol)

References

Details

(Keywords: verified1.9.1)

Attachments

(2 files, 3 obsolete files)

test case
(deleted), text/html
Details
(rev 2b) with requested change [Checkin: Comment 22]
(deleted), patch
Details | Diff | Splinter Review
Attached file test case (deleted) — 
If you set the background of the <html> element to be transparent, e.g. with

 html { background-color: rgba(0,0,0,0) }

this will initially paint the same way it would if you had set the background to be white, but if you then resize the window, garbage will appear everywhere that isn't covered by the <body> element's box.   I often seem to get an insensitive duplicate of the browser chrome, or some mangled version of it, or smears of the body content.

This is in both FF3.0.x and trunk on Linux (i.e. gtk backend).  Should be tried on other platforms and with FF2 in case it's a regression.
A slight clarification to my original report - the bug does NOT manifest with background-color:transparent, only with background-color:rgba(0,0,0,0).

CSS2.1 §14.2 has this to say:

# The background of the root element becomes the background of the canvas
# and covers the entire canvas, anchored (for 'background-position') at 
# the same point as it would be if it was painted only for the root
# element itself. The root element does not paint this background again.

# For HTML documents, however, we recommend that authors specify the
# background for the BODY element rather than the HTML element. For
# HTML documents whose root HTML element has computed values of 'transparent'
# for 'background-color' and 'none' for 'background-image', user agents must
# instead use the computed value of those properties from that HTML element's
# first BODY element child when painting backgrounds for the canvas, and must
# not paint a background for that BODY element. Such backgrounds must also be
# anchored at the same point as they would be if they were painted only for
# the root element. This does not apply to XHTML documents. 

Either this rule should be extended to background-color:rgba(0,0,0,0) on the HTML element, or we should behave as-if there is a white surface underneath the canvas in that case.  Also, I wonder what we're supposed to do for XHTML documents.
Summary: HTML element with transparent background produces garbage on screen → HTML element with rgba(0,0,0,0) background produces garbage on screen
What happens with an alpha value between 0 and 1?
Throw me a patch that fixes it around here:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp#1309
by compositing the style background color onto DefaultBackgroundColor if necessary
> What happens with an alpha value between 0 and 1?

The color you specify, at the alpha you specify, is composited on top of the garbage.

> Throw me a patch that fixes it around here: (url)
> by compositing the style background color onto DefaultBackgroundColor if
> necessary

It's not immediately obvious to me what the right conditions are - I'll look at it more tomorrow.  What's a "glass window"?
That's Aero glass mode. Basically an alternative kind of transparent window.

I can work on this myself but I can't do it right now.
Confirming. A slightly different bug is visible on Windows: the background turns black when the window is resized.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch (rev 1) fix for bug (obsolete) (deleted) — — Splinter Review 
This turned out to be easy to fix once I was awake enough to understand what the code roc pointed at was doing.

There's a reftest in here, but when run against a browser with the bug, it passes, even though I can see display garbage in the window that appears.  I imagine this is because the display buffer the reftest harness looks at, doesn't suffer corruption, and there may be nothing we can do about that.
Assignee: nobody → zweinberg
Status: NEW → ASSIGNED
Attachment #336898 - Flags: review?(roc)
I think we could reftest this by initializing the reftest canvas to some kind of pattern (or even just a strange color).
Is it guaranteed that mBackgroundColor is RGBA(0,0,0,0) if the NS_STYLE_BG_COLOR_TRANSPARENT flag is set?

I kinda wonder why we even have that flag since it should be equivalent to RGBA(0,0,0,0).
The flag predates RGBA colors; bug 372781 is about removing it and some other things.
It looks like nsRuleNode.cpp (which is the only place that sets mBackgroundColor to an interesting value) isn't scrupulous about making sure that the transparent flag and mBackgroundColor are consistent.  Also the assertion in my patch is wrong; the user might've gone and changed the DefaultBackgroundColor to something with transparency.

Will revise tomorrow.  Might get irritated and zap NS_STYLE_BG_COLOR_TRANSPARENT per bug 372781.
I'm not really sure what the desired behaviour is if the user specifies a translucent color for the background. We could either set its alpha channel value to 255 or composite it onto white or something.
See also bug 334552.
(In reply to comment #8)
> I think we could reftest this by initializing the reftest canvas to some kind
> of pattern (or even just a strange color).

I tried changing this bit of reftest.js...

    /* XXX This needs to be rgb(255,255,255) because otherwise we get
     * black bars at the bottom of every test that are different size
     * for the first test and the rest (scrollbar-related??) */
    var win = gBrowser.contentWindow;
    canvas.getContext("2d").drawWindow(win, win.scrollX, win.scrollY,
                                       canvas.width, canvas.height,
                                       "rgb(255,255,255)");

to pass rgba(0,0,0,0).  This does cause it to fail my test case, but not the way I was expecting it to -- in reftest-analyzer.xhtml, the two images are visually identical, and the difference highlight doesn't make sense.  [Are there any other analysis tools out there?  Too often, I can't tell from reftest-analyzer.xhtml what the problem actually is.  I am very close to frustrated enough to write my own.]

Also, the change does *not* produce black bars at the bottom of the tests as far as I can tell, but does cause a handful of additional failures which might actually be further instances of the same bug!

Finally, I traced the code of the drawWindow method down into the pres shell and I'm not convinced it *can* catch this sort of "we didn't paint the entire root frame" bug - it doesn't blit out of the actual window, it builds a whole new display list and does the painting all over again.
Depends on: 453916
Additional data: when I got home I wrote a little script to extract reftest dump images as PNG files (unlike, apparently, everything in layout/tools/reftest :-P) and it was then immediately obvious on looking at them in a real image viewer that my test case is now being failed in a meaningful way (and in fact allows me to simplify it) -- most of the test image is partially transparent.  It was not so obvious what had happened to the other test cases that failed; I shall have to actually look at the source of the tests (profound sigh) before I can say whether these are real bugs now exposed or what.  However, my dire suspicions about the implementation of drawWindow are probably moot.
Further experimentation with changes to reftest.js and an analyzer of my own invention have revealed two things.  First, if you tell drawWindow not to paint the canvas white, reftests bugs/438987-2[abc].html fail with exactly the problem reported in this bug -- the background is painted as a semitransparent color over whatever happens to be in the canvas already -- the rendering results from a previous test cycle, for instance.  This will be fixed by the revised patch for this bug when I get around to coding it, and means maybe we can just throw out the test case I wrote and its silly JavaScript display manipulations.

Second, under some circumstances the display list that drawWindow builds and then executes does not cover the entire canvas surface, leaving an unpainted strip at either the right or the bottom of the canvas.  I do not see anything special about the test cases where this occurs; in particular, they do *not* involve scrolling the window. I am inclined to think it a bug in drawWindow if it doesn't always paint every pixel of a rectangle sized according to its arguments, but that goes into pres shell territory about which I know nothing.
Blocks: 454349
No longer blocks: 454349
The main change here is, post bug 453916, we no longer have to worry about the NS_STYLE_BG_COLOR_TRANSPARENT flag and mBackgroundColor being out of sync.  Also I replaced the assertion that the default background color is opaque with a second composition, onto white, if if isn't.  (The default background color can be modified via hidden prefs, so we can't count on its being opaque.)

I'm not sure the test cases are done.  I started down this road because dbaron wondered in bug 450652 whether it was possible to try to paint a rounded-corner background for the root element, and while I haven't yet been able to make that happen, I haven't yet found code that prevents it, either.
Attachment #336898 - Attachment is obsolete: true
Attachment #337825 - Flags: review?
Attachment #336898 - Flags: review?(roc)
Attached patch (rev 2a) testcases dropped (obsolete) (deleted) — — Splinter Review 
I've come to the conclusion that if it's possible to force the root element to be drawn with rounded corners, that's a separate bug, and therefore this patch doesn't actually need its own test cases - if landed together with bug 454349, the existing test case bugs/438987-2[abc].html will suffice.
Attachment #337825 - Attachment is obsolete: true
Attachment #338191 - Flags: review?
Attachment #337825 - Flags: review?(roc)
Comment on attachment 338191 [details] [diff] [review]
(rev 2a) testcases dropped

+      if (NS_GET_A(backColor) < 255)
+        backColor = NS_ComposeColors(NS_RGB(255,255,255), backColor);

Make this unconditional, it saves one line of code
Attachment #338191 - Flags: superreview+
Attachment #338191 - Flags: review?
Attachment #338191 - Flags: review+
Done.
Attachment #338191 - Attachment is obsolete: true
Keywords: checkin-needed
(note to anyone trying to land this: you gotta do bug 453916 first, and that patch is not ready yet.)
http://hg.mozilla.org/mozilla-central/rev/d7c555ce0fc7
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b1
Shouldn't this be marked FIXED now?
sorry, i'm being slow today.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Depends on: 455093
See bug 455093 -- looks like either this or bug 453916 regressed some reftests on seamonkey/macOS
No longer depends on: 455093
Attachment #338245 - Attachment description: (rev 2b) with requested change → (rev 2b) with requested change [Checkin: Comment 22]
Hrm, this might have caused bug 469170.
We should back this out on trunk and 1.9.1. Bug 467459 fixed this the right way on trunk and in 1.9.1.
Keywords: checkin-needed
per discussion on IRC, a straight backout is not exactly right.  The right patch will be developed in bug 473398.
Depends on: 473398
Keywords: checkin-needed
Keywords: fixed1.9.1
pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d7c555ce0fc7

verified FIXED on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090416 Minefield/3.6a1pre ID:20090416030845

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090417 Shiretoko/3.5b4pre ID:20090417030722
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: