Chris Mills [:cmills] Reporter
	Description • 8 years ago

I wrote a background-attachment demo to go along with one of my recent MDN Learning Area articles:

http://mdn.github.io/learning-area/css/styling-boxes/backgrounds/background-attachment.html

In latest Nightly/Dev Edition, the boxes with fixed and scroll applied seem to work ok, but the box with local applied doesn't work properly, afaics. The background should scroll as the element content scrolls, but it doesn't — it works the same as background-attachment: scroll.

The demo seems to work correctly in latest Chrome and Firefox release.

	Timothy Nikkel (:tnikkel) Assignee
	Comment 1 • 8 years ago

Guessing this is apz related based on nightly/dev edition failing, but release working.

	Botond Ballo [:botond]
	Comment 2 • 8 years ago

Confirmed - APZ has regressed the behaviour of background-attachment:local. It looks like this property may need compositor support, similar to how background-attachment:fixed does.

	Markus Stange [:mstange]
	Comment 3 • 8 years ago

It looks like Layout needs to handle layerization of the background image in much the same way as it handles the caret in scrollable textboxes - by applying a scroll clip to the nsDisplayBackgroundImage that is associated with the scroll frame (so that APZ sees the right frame metrics), and by setting the clip of that scroll clip to the background clip region. It'll also need to enlarge the bounds of the display item to the display port of the scroll frame.

	Markus Stange [:mstange]
	Comment 4 • 8 years ago

Oh, I was wrong about how background-clip interacts with background-attachment: local. The clip isn't fixed to the scroll frame; it's moving along with the contents. This makes things easier because we don't need to create a special scroll clip.
It looks like we should be able to just put the background into the same layer as the scrolled content, except if there is something from the frame's decoration that needs to go in between the background and the content, for example an inset box shadow.

	Markus Stange [:mstange]
	Comment 5 • 8 years ago

Matt, do you think something like this would work? 1. In nsDisplayBackgroundImage::AppendBackgroundItemsToTop, if aFrame is an nsIScrollableFrame, skip any layers with attachment:local. 2. In ScrollFrameHelper::BuildDisplayList, once the scroll clip and dirty rect have been set up for the scrolled contents, create all the background image display items that we skipped before. 3. afterwards, sort the display list so that the background image display items are in the right order.

	Timothy Nikkel (:tnikkel) Assignee
	Comment 6 • 8 years ago

Could we just determine the scrollclip and dirty rect before we create the items for the attachment local items to avoid having to sort the list afterwards?

	Markus Stange [:mstange]
	Comment 7 • 8 years ago

We also need to know the animated geometry root. Which might change after we've descended into the scroll frame's children due to scroll handoff.

	Matt Woodrow (:mattwoodrow)
	Comment 8 • 8 years ago

As discussed on irc, it might be easiest to build the items at the normal time, but track them (on the builder?), so we can re-visit them once the correct clip is in place and update the clip/dirty rect.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 9 • 8 years ago

Doesn't look like we'll get to this in 47. From comment 8, it sounds like we do have a plan for tackling this, but we need somebody to claim ownership.

I'm also tagging this as a regression from APZ because it works fine without APZ.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 10 • 8 years ago

Parking with Timothy for now, although he may not have cycles to get to it this month. If anybody else wants to steal it feel free.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 11 • 8 years ago

Bumping this to fix-optional for 48 since this is a relatively new CSS feature and we haven't run into any actual websites using it or having this problem.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 16 • 8 years ago

Too late for 48/49.

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 17 • 8 years ago

In comment 11 some months  ago we didn't know of any examples of this in the real world - has that changed? 
Marking fix-optional for 51, we could still take a fix in early beta 51 (next few weeks) but it doesn't seem crucial.

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 7 years ago

Looking pretty unlikely we're going to get a fix for this in time for backport to 51/52, but leaving them set to fix-optional just in case.

	Andrew Overholt [:overholt]
	Comment 19 • 7 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> Bumping this to fix-optional for 48 since this is a relatively new CSS
> feature and we haven't run into any actual websites using it or having this
> problem.

Is this still true, kats?

	Timothy Nikkel (:tnikkel) Assignee
	Comment 20 • 7 years ago

Here is a work in progress that I got working a long time ago. It includes some stuff for testing (making all scroll frames active all the time). It's probably bitrotten.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 21 • 7 years ago

(In reply to Andrew Overholt [:overholt] from comment #19)
> Is this still true, kats?

As far as I know, yes. The only person who's reported it that I can recall is the one in bug 1276131, and that was a reduced test case on codepen, not a live website.

	Andrew Overholt [:overholt]
	Comment 22 • 7 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #21)
> (In reply to Andrew Overholt [:overholt] from comment #19)
> > Is this still true, kats?
> 
> As far as I know, yes. The only person who's reported it that I can recall
> is the one in bug 1276131, and that was a reduced test case on codepen, not
> a live website.

Thanks, that means I'll keep the fix-optional for 53 and 54.

	Sylvestre Ledru [:Sylvestre]
	Comment 23 • 6 years ago

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

	Sylvestre Ledru [:Sylvestre]
	Comment 24 • 6 years ago

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information