Incorrect rendering of background-attachment:fixed when opacity<1

VERIFIED FIXED

Status

()

Core
Layout
VERIFIED FIXED
7 years ago
4 years ago

People

(Reporter: gadjo, Assigned: roc)

Tracking

({regression, testcase})

Trunk
regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [hardblocker][retainedlayers][has patch])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre
Build Identifier: 

When element has background-attachment:fixed, background-repeat: repeat-x and opacity<1 it is randomly (?) repeated also in y axis. Testcase and screenshots will be attached

Reproducible: Always

Steps to Reproduce:
1. Open the testcase
2. Scroll page up/down
Actual Results:  
Background is repeated in y-axis

Expected Results:  
Background should be repeated only in x-axis
(Reporter)

Updated

7 years ago
Duplicate of this bug: 618723
(Reporter)

Comment 2

7 years ago
Created attachment 497177 [details]
test case

Scroll the page up and down to see the bug
(Reporter)

Comment 3

7 years ago
Created attachment 497179 [details]
screenshot

Upper part shows the expected behavious, the lower one shows the behaviour after scrolling down and up
After switching to other tab and then switching back to the test case tab it looks ok
(Reporter)

Comment 4

7 years ago
More info - the bug doesn't show after removing "div { opacity: 0.5 }" or "body { margin: 0 }" rule
Version: unspecified → Trunk
(Reporter)

Updated

7 years ago
Keywords: regression
Keywords: testcase
I also see this on Linux.  Probably a regression from layers work.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All

Comment 6

7 years ago
Regression window(cached hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/92339b84d089
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100715 Minefield/4.0b2pre ID:20100715145415
Fails:
http://hg.mozilla.org/mozilla-central/rev/e1d7fd5255fd
Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100715 Minefield/4.0b2pre ID:20100715152722
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=92339b84d089&tochange=e1d7fd5255fd
Blocks: 564991
Assignee: nobody → tnikkel
blocking2.0: ? → final+
Whiteboard: [hardblocker]
This reminds me very much of bug 612459.  That bug has a regression range but this one has a better testcase.
Whiteboard: [hardblocker] → [hardblocker][retainedlayers]
Assignee: tnikkel → roc
Created attachment 503060 [details] [diff] [review]
fix

This fixes it. I'm still trying to write a reliable test.
Attachment #503060 - Flags: review?(tnikkel)
Whiteboard: [hardblocker][retainedlayers] → [hardblocker][retainedlayers][needs review]
Looks like you uploaded the wrong patch here.
Created attachment 503078 [details] [diff] [review]
real patch
Attachment #503060 - Attachment is obsolete: true
Attachment #503078 - Flags: review?(tnikkel)
Attachment #503060 - Flags: review?(tnikkel)

Comment 11

7 years ago
I've got 2 reliable tests for you:

1.  Using the Stylish extension, create a user style with this simple CSS:

    @namespace url(http://www.w3.org/1999/xhtml);
    @-moz-document domain("facebook.com") 
    {
    .UIStory {background:transparent !important; opacity:0.25 !important}

    body{
    background-image: url("http://i43.tinypic.com/2jb0c3d.jpg") !important;
    background-repeat:no-repeat !important;
    background-position:center !important;
    background-attachment:fixed !important;
    }
    }


Then visit any Facebook wall with this user style activated... other portions of the page will be copied under the posts.

2.  Same thing but using GreaseMonkey (to prove it's not Stylish causing it)...  Create a GM script with this content:

// ==UserScript==
// @name           TESTER
// @namespace      http://userstyles.org
// @description    overlay test
// @include        http://facebook.com/*
// @include        https://facebook.com/*
// @include        http://*.facebook.com/*
// @include        https://*.facebook.com/*
// ==/UserScript==
(function() {
var css = ".UIStory {background:transparent !important; opacity:0.25 !important}\nbody{background-image: url(http://i43.tinypic.com/2jb0c3d.jpg) !important;background-repeat:no-repeat !important;background-position:center !important;background-attachment:fixed !important;}";
if (typeof GM_addStyle != "undefined") {
	GM_addStyle(css);
} else if (typeof PRO_addStyle != "undefined") {
	PRO_addStyle(css);
} else if (typeof addStyle != "undefined") {
	addStyle(css);
} else {
	var heads = document.getElementsByTagName("head");
	if (heads.length > 0) {
		var node = document.createElement("style");
		node.type = "text/css";
		node.appendChild(document.createTextNode(css));
		heads[0].appendChild(node); 
	}
}
})();


and then visit a Facebook wall with the script active... same thing, copies of portions of the page underlying posts.

Updated

7 years ago
Whiteboard: [hardblocker][retainedlayers][needs review] → [hardblocker][retainedlayers][needs review][has patch]
Comment on attachment 503078 [details] [diff] [review]
real patch

>background-attachment:fixed display items rendered via temporary layer managers can't have their scrolling taken care of via the layer system

Here is a dumb question: why not?
Because they don't have retained layers whose offsets we can manipulate to scroll.
Do they not get assigned to thebes layers the same way that display items without temporary layer managers do?
They do, but those layers aren't retained.

The problem here is in InvalidateFixedBackgroundFramesFromList:
      if (item->IsFixedAndCoveringViewport(aBuilder)) {
        // FrameLayerBuilder takes care of scrolling these
The idea is that such items will have been placed in their own layer, and FrameLayerBuilder will set the offset on the layer to account for scrolling (typically by making it invariant when we scroll!), and that will take care of scrolling without us needing to invalidate any part of a ThebesLayer.

But when an item is drawn through a temporary LayerManager, that assumption is not true. The item has been drawn into some ThebesLayer and what it rendered will be scrolled along with the item that induced the temporary layer manager (e.g., an element with a transform not marked active).
(In reply to comment #15)
> They do, but those layers aren't retained.

The layers inside the temp layer manager aren't retained, but the result of drawing the entire temp layer manager is retained in whatever thebes layer the item was assigned to.

> The problem here is in InvalidateFixedBackgroundFramesFromList:
>       if (item->IsFixedAndCoveringViewport(aBuilder)) {
>         // FrameLayerBuilder takes care of scrolling these
> The idea is that such items will have been placed in their own layer, and
> FrameLayerBuilder will set the offset on the layer to account for scrolling
> (typically by making it invariant when we scroll!), and that will take care of
> scrolling without us needing to invalidate any part of a ThebesLayer.
> 
> But when an item is drawn through a temporary LayerManager, that assumption is
> not true. The item has been drawn into some ThebesLayer and what it rendered
> will be scrolled along with the item that induced the temporary layer manager
> (e.g., an element with a transform not marked active).

I don't see why the non-temp layer manager case doesn't apply to the temp layer manager case. In both cases the result of drawing the item ends up in a thebes layer with the same active scrolled root. The transform on this layer should be adjusted the same if it contains items that have or do not have temp layer managers. What am I missing?
ContainerState::ProcessDisplayItems has this:

      nsIFrame* activeScrolledRoot =
        nsLayoutUtils::GetActiveScrolledRootFor(f, mBuilder->ReferenceFrame());
      if (item->IsFixedAndCoveringViewport(mBuilder)) {
        // Make its active scrolled root be the active scrolled root of
        // the enclosing viewport, since it shouldn't be scrolled by scrolled
        // frames in its document. InvalidateFixedBackgroundFramesFromList in
        // nsGfxScrollFrame will not repaint this item when scrolling occurs.
        nsIFrame* viewportFrame =
          nsLayoutUtils::GetClosestFrameOfType(f, nsGkAtoms::viewportFrame);
        NS_ASSERTION(viewportFrame, "no viewport???");
        activeScrolledRoot =
          nsLayoutUtils::GetActiveScrolledRootFor(viewportFrame, mBuilder->ReferenceFrame());

So in the simple retained case, the fixed item ends being drawn into a ThebesLayer whose active scrolled root is the viewport. But in the temp-layer-manager case, the fixed item probably ends up being drawn into a ThebesLayer whose active scrolled root is NOT the viewport (it's probably the root scrolled frame).
The temp layer manager will actually build a layer tree where the fixed item is assigned to a ThebesLayer whose active scrolled root is the viewport, but then that entire layer subtree is mashed together and drawn into the single ThebesLayer containing the nsDisplayTransform (or whatever), whose active scrolled root is not the viewport.
Ah ok, so the item with the temp layer manager has the scrolled frame as active scrolled root, but an item under that item has the viewport as its active scrolled root.
Attachment #503078 - Flags: review?(tnikkel) → review+
Comment on attachment 503078 [details] [diff] [review]
real patch

Doesn't seem like it would be too hard to write a reftest for this.
Whiteboard: [hardblocker][retainedlayers][needs review][has patch] → [hardblocker][retainedlayers][needs landing][has patch]
http://hg.mozilla.org/mozilla-central/rev/84cebd5b0f93
http://hg.mozilla.org/mozilla-central/rev/1431568b3e1b
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [hardblocker][retainedlayers][needs landing][has patch] → [hardblocker][retainedlayers][has patch]
Verified on Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.