582146 - Tall iframe breaks painting

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b3pre) Gecko/20100726 Minefield/4.0b3pre

When Firefox navigates to this testcase, it paints nothing -- not even white.  This may be a spoofing risk, both for the testcase and the other page.

Steps:
1. Load http://www.mozilla.com/
2. Load the testcase (by pasting its URL into the address bar)

Result: Content area still looks like mozilla.com.
Expected: Content area should appear white.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That's disturbing. Timothy, do you want to try to fix this?

Comment 3

[Damon Sicore (:damons)]

Who owns this?  Need an owner ASAP.

Comment 4

[Timothy Nikkel (:tnikkel)]

I can reproduce what I think is the same issue on Linux: it just paints black.

Looks like this was caused by bug 572613 (retained layers basically).

Comment 5

[Timothy Nikkel (:tnikkel)]

Changing
  aCtx->FillRect(bgClipRect)
in nsDisplayCanvasBackground::Paint to fill the mVisibleRect instead seems to fix this bug. So I guess the bgClipRect is overflowing and causing it to not paint.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Just intersect the bg clip area with the visible rect and draw that.

I don't know why nsThebesRenderingContext::FillRect doesn't draw anything when given such a large rect because it has code specifically to deal with that situation, and it executes correctly in this case. The problem must lie further down into the gfx code.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think we should find the underlying bug. The existing code should work.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Setting a breakpoint on _cairo_error might be useful.

Comment 9

[Timothy Nikkel (:tnikkel)]

A breakpoint on _cairo_error doesn't seem to be hit.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Easy fix! We were clamping cairo coordinates to 8388608. However, converting 8388608 to cairo_fixed_t actually wraps around. 8388607 is the largest coordinate that does not wrap around, so clamp to that instead.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 476705 [details] [diff] [review]
fix

This constant would be more meaningful if it were a written in hex and casted to a double.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/8ddb7ffe0587
Fixed test and marked it as having an assertion:
http://hg.mozilla.org/mozilla-central/rev/a8212bca66fb
http://hg.mozilla.org/mozilla-central/rev/5a41a70eb631