The code in nsCSSRendering::PaintBackground that tries to ensure an opaque background for the canvas frame is unnecessary now that PresShell::Paint does the same work. This was noted during the discussion of bug 469170 but does not fix that bug.

We should just do a straight backout of bug 453566. See my comment in bug 469170. In particular this block + if (isCanvas) { + nsStyleBackground canvasColor(*color); + nsIViewManager* vm = aPresContext->GetViewManager(); + vm->SetDefaultBackgroundColor(canvasColor.mBackgroundColor); } is not wanted.

That block predates bug 453566, though! And a straight backout would leave us with special casing for frames where isCanvas is true and the alpha component of the background color is exactly zero, which I don't think makes sense anymore either. If what you're saying is PaintBackground no longer needs to special case canvas frames at all, I believe you, but a straight backout of 453566 won't get us there.

ok. I was wrong in comment #2 though, we need to keep that code. So can you write a patch that removes the special casing of exactly-zero but otherwise reverts 453566?

Comment on attachment 356736 [details] [diff] [review] patch This patch is almost right, but we don't want to put a non-opaque color into SetDefaultBackgroundColor. So I'd just make the call to SetDefaultBackgroundColor conditional on the alpha channel == 255. For transparent backgrounds, that's equivalent to the 1.9.0 code --- we're just leaving the current default-background-color as-is, which makes sense.

So, like this?

Pushed http://hg.mozilla.org/mozilla-central/rev/5f9daba6c730

Comment on attachment 357189 [details] [diff] [review] (rev 2) only set view manager default if opaque We need this on 1.9.1 because it fixes bug 469170

Actually, since 469170 is blocking and this fixes it, we shouldn't beat about the bush

I'm not sure this fixes it all by itself...

... ok, unmodified trunk as of now does pass the test in bug 469170.

This causes the regression in bug 474201, so I'm not sure landing it in 1.9.1 is the right idea at this time.

Comment on attachment 357189 [details] [diff] [review] (rev 2) only set view manager default if opaque This is a blocker, so it doesn't need approval. Don't forget to land the fix for bug 474201 on the branch as well.

(In reply to comment #12) > This causes the regression in bug 474201, so I'm not sure landing it in 1.9.1 > is the right idea at this time. And while that bug's now been fixed, *its* fix apparently caused a number of other regressions, too. So I'm guessing it's still probably not a good idea to land this on 1.9.1 right now.