Tall iframe breaks painting

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: jruderman, Assigned: roc)

Tracking

({testcase})

Trunk
x86
macOS
testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [sg:low spoof])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
Created attachment 460415 [details]
testcase
blocking2.0: --- → ?
That's disturbing. Timothy, do you want to try to fix this?
blocking2.0: ? → betaN+
Who owns this?  Need an owner ASAP.
Assignee: nobody → 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).
Blocks: 572613
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.
Created attachment 461916 [details] [diff] [review]
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.
Attachment #461916 - Flags: review?(roc)
I think we should find the underlying bug. The existing code should work.
Setting a breakpoint on _cairo_error might be useful.
A breakpoint on _cairo_error doesn't seem to be hit.
Assignee: tnikkel → nobody
Created attachment 476705 [details] [diff] [review]
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.
Assignee: nobody → roc
Attachment #461916 - Attachment is obsolete: true
Attachment #476705 - Flags: review?(jmuizelaar)
Attachment #461916 - Flags: review?(roc)
Whiteboard: [sg:low spoof] → [sg:low spoof][needs review]
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.
Attachment #476705 - Flags: review?(jmuizelaar) → review+
Whiteboard: [sg:low spoof][needs review] → [sg:low spoof][needs landing]
Whiteboard: [sg:low spoof][needs landing] → [sg:low spoof]
Flags: in-testsuite+
(Reporter)

Updated

7 years ago
Blocks: 334359

Updated

4 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.