68499 - CSS1: Background-attachment:fixed buggy for elements other than BODY

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Ian Graham]

It appears that background-attachment: fixed works on elements that
are descendents of BODY _only_ if BODY itself takes a fixed background.
If BODY does not have fixed background (i.e, no background or a 
scrolling one), then the fixed background of a descendant does not
scroll, and is in fact "smeared" if it is scrolled past the upper and
lower viewport boundaries.

The referenced URL (http://www.java.utoronto.ca/NS5-bugs/attach.html)
links to three example document illustrating this behavior.

The expected behavior is that a background with background-attachment:fixed 
should be fixed independent of the background properties of other elements.

This bug was observed in Mozilla 0.7 [Build: 2001010901]

Note: This bug is likely related to previously fixed bug 40217: "CSS1 
background-attachment:fixed does not fix the background" 
(http://bugzilla.mozilla.org/show_bug.cgi?id=40217)

Comment 1

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

I don't see any smearing - I just see the background not being fixed.  Perhaps
that's because I'm using roc's new viewmanager.

Comment 2

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

I see smearing.

Comment 3

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

But I'm using a pretty old build. Hmm.

Comment 4

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

OK, new build. The image is actually drawn in the correct fixed position if you 
force an expose ... it's just not redrawing at the appropriate time.

Comment 5

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

OK, this is happening because the DONT_BITBLT flag is set on the view for the 
inner block element, but since the scrolling is being done by the body block 
element, it sees that its own DONT_BITBLT flag is not set and happily bitblts.
The solution is to make a widget for background-attach: fixed elements.

Comment 6

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

OK, I have a patch. The patch makes nsViewManager respect DONT_BITBLT flags set
on any child (widget-bearing) view of the view being scrolled. We still use fast
platform scrolling for the scrolled widget, but we make sure to invalidate any
children which have DONT_BITBLT set.

The patch also forces the view for a "background-attachment: fixed" frame to
have a widget.

Reviews, etc, welcome.

Comment 7

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

Stealing bug

Comment 8

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

Accepting

Comment 9

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

Attachment: Changes to nsViewManager and nsHTMLContainerFrame

Comment 10

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

Would invalidating the descendants (or did you really mean children?) with
DONT_BITBLT without forcing views for 'background-attachment: fixed' elements to
have widgets fix the problem?  What are the disadvantages of making them have
widgets?  (What views have widgets right now?)  I think (although I"m not sure)
that I remember comments from when 'position: fixed' was originally implemented
that it would have been nicer not to require widgets for 'position: fixed'.

Comment 11

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

I mean "descendants". We might have a situation where none of the immediate 
child views have DONT_BITBLT but some of their children do.

Hmm. I thought I had a good reason to make them widgets but I can't think of it 
now :-). It should be doable without special widgets. I'll try.

Comment 12

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

Attachment: Better patch --- changes nsViewManager only

Comment 13

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

That should do the trick!
Now let's get the patch in before it rots.

Comment 14

[Kevin McCluskey (gone)]

patch looks good.
r=[e-mail]

minor nit: shouldn't aRepaintOnlyUnbittableViews be called
aRepaintOnlyUnblittableViews?

Comment 15

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

Er, yeah. I'll check in the correct spelling.

Comment 16

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

Hmm, who should be the super-reviewer? What's buster's status?

Comment 17

[buster]

buster has left the building

well, mostly.  anyway, waterson is your source for sr.  attinasi should get sr
privelages, if he hasn't already.l

Comment 18

[Hixie (not reading bugmail)]

roc: dude. 4 days from bug filing to awaiting sr=. you rock! :-)

attinasi/waterson: can we have an sr=?

Comment 19

[Marc Attinasi]

sr=attinasi

Comment 20

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

Fix checked in.

Comment 21

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

This isn't actually fixed until the new view manager is turned on. Adding
dependence on bug 39621 and reopening.

Comment 22

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

The new view manager is checked in and enabled, so this bug is now fixed.

Comment 23

[Ian Graham]

Verified fixed in 30/March nightly build (build 2001033020).
However, I note that when only the background of a non-body 
element is fixed, as in
http://www.java.utoronto.ca/NS5-bugs/bg-attach-ponly.html, 
then the fixed background 'jumps' vertically a bit when the 
page is scrolled.  So it works, but there's some redraw 
problem affecting this case. 

Comment 24

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

There are many situations where we get jumpy redraws. Here it's because we first 
do a screen-to-screen bitmap copy for fast scrolling, then we do a redraw to fix 
up the display. In other situations, it's because updating the screen requires 
repainting of more than one widget, which can only be done one widget at a time. 
(This latter one is particularly noticeable with fixed-position elements.) 
Fixing these would require a significantly different rendering architecture with 
1) minimal usage of native widgets, preferably one widget per page
and
2a) platform support for copying bits from the screen to an off-screen rendering 
buffer
or
2b) explicitly retaining the rendered bits for each page in an off-screen buffer 
so we can copy from them later

On some platforms we might be able to arrange for some sort of synchronized 
update of a lot of native widgets at once, including scroll operations, but I 
don't even know how to do that for Windows, let alone the platforms I'm less 
familiar with.

In other words, this is a tough one.

Comment 25

[Hixie (not reading bugmail)]

Robert: 2b would fit in very well with the proposed fix for bug 65701.

Comment 26

[Ian Graham]

I can see how this is a tough problem. But it's clear that the
performance hit relative to platform-specific renders (IE, etc.)
can be significant, and thus a real impediment to use of 
Mozilla-based products on slower hardware. It's also an easy 
'club' with which Mozilla will be bashed by a certain Unnamed 
Browser  Vendor ;-)  

So, performance analysis and re-engineering is probably 
called for, although I wouldn't hazard a timeline for it.

Comment 27

[Hixie (not reading bugmail)]

VERIFIED using the provided test cases