296838 - drawWindow() doesn't draw elements with position: fixed;

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[(not currently active) Ted Mielczarek]

This was found viewing this website: http://www.brentanos.fr/  (a terrible mess
of html).

Testcase in a sec.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: position: fixed html

HTML using position: fixed

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: canvas attempting to draw the previous attachment (uses enablePrivilege)

This example uses the previous attachment in an iframe and attempts to render
it to a canvas.  It uses enablePrivilege('UniversalBrowserRead').

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: same as above, but with an iframe inside an iframe

Strangely enough, it works if the fixed content is inside an iframe in the
window you're trying to draw.

Comment 4

[Asaf Romano (gone)]

I see both rects in all testcase.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050606
Firefox/1.0+

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: Broken rendering

To clarify, in these testcases, what you should see is two identical images,
with a red rectangle overlapping a green rectangle.  If you see a large blank
space on the left side, then the canvas isn't working right.  If you see just a
green rectangle on the left side, then drawWindow() isn't working right.

Attached is an image of incorrect rendering on the first testcase (attachment
185477 [details]).

Comment 6

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

This is quite tough to fix.

Suppose you have a document with "position:fixed" elements, and you scroll down.
Then you call drawWindow. What should you get? An image of the document as it
would be if it was scrolled to the top? Or an image of the document scrolled down?

Comment 7

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

I'll make it so that you get the whole document, but with the fixed elements at
the position in the document where they appear visually. So if you've scrolled
down, the fixed elements appear down the document.

I have a handle on fixing this, it's just a *little* more complicated than I
expected. I'll post a patch on Monday.

Comment 8

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

Attachment: fix

This fixes the problem. It's a matter of making sure scrollbars on the root
view aren't rendered, and that the root scrollable view is not used to clip any
views. We also adjust the rectangle used for rendering to compensate for the
amount the viewport is scrolled, and translate the rendered output to the
correct position.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

rubber-stamp r+sr=dbaron, although aSuppressClip, aSuppressScrolling, and
suppressScrolling seem like odd names for views (they sound like booleans).

Comment 10

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

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

fixes a significant drawWindow issue, very low risk to non-drawWindow users

Comment 11

[chris hofmann]

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

a=chofmann

Comment 12

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

checked in

Comment 13

[jag (Peter Annema)]

This seems to have increased Tp on btek by about 4 to 5 ms.

Comment 14

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

why do you say that?

Comment 15

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

oops, sorry, I was looking at the wrong thing

Comment 16

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

Attachment: fix #2

This might ameliorate the Tp effect. The patch changed our behaviour on the
regular rendering paths from passing aTopView to GetClippedRect() to passing
null. This changes it back to passing aTopView. I can't see why that would have
much Tp effect; aTopView is usually the topmost view, no parent or zparent, so
nothing extra really happens in ApplyClipRect. When aTopView is not the topmost
view, it's almost always a popup, so we usually break out of the loop around
the same time. But I can't see any other possibility for the slowdown.

The weird thing is that before the offscreen rendering stuff landed a few
months ago, the loop condition in ApplyClipRect was PR_TRUE --- the same
behaviour that we get now passing null. So that checkin would have sped us up
by this amount. I think stopping at aTopView is correct though.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 187093 [details] [diff] [review]
fix #2

Rubber-stamp r+sr=dbaron; sorry for the delay.

Comment 18

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

Comment on attachment 187093 [details] [diff] [review]
fix #2

fixes drawWindow bug, something that was already approved, but this has some
slight changes to reduce Tp impact (hopefully).

Comment 19

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

checked in