Ada [:adalucinet] Reporter
	Description • 8 years ago

Affected builds: latest Aurora 46.0a2 and Nightly 47.0a1 (from 2015-01-26), e10s enabled/disabled

Affected platforms:
* Mac OS X 10.9.5
* Windows 7 64-bit
* Ubuntu 15.04 64-bit	

Steps to reproduce:
1. Launch Firefox and navigate to https://gaming.youtube.com/
2. Select any video - E.g.: https://gaming.youtube.com/watch?v=gCISR0T3grE
3. Scroll down/up and repeat.

Expected results: The page is properly displayed.

Actual results: Repainting issues are visible in the video area.

Additional notes:
1. Not reproducible with 44.0RC (note that layers.async-pan-zoom.enabled is set to false by default).
2. With layers.async-pan-zoom.enabled set to false, the repainting is no longer visible, but scrolling is not smooth at all.
Regression range:
(m-c)
Last good revision: 8e5c888d0d89 (2015-07-22) 
First bad revision: 1f77b78797d6 (2015-07-23) 
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8e5c888d0d89&tochange=1f77b78797d6 

(m-i)
Last good revision: 6ad92b48085b5ec5532f14e0b9e3c5eb15ad9fa8
First bad revision: cfc312d8ef205bcb6a58b840459a4ccd922dcf88
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6ad92b48085b5ec5532f14e0b9e3c5eb15ad9fa8&tochange=cfc312d8ef205bcb6a58b840459a4ccd922dcf88

Regressed by: cfc312d8ef20 Kartikaya Gupta — Bug 1157746 - Enable APZ on OS X desktop nightly builds. r=mstange

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

Can you provide a screenshot of the repainting issues? Are they transient or permanent (i.e. do they fix themselves, or do they remain after scrolling has stopped)?

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

Never mind, I can reproduce this - it's another case where they are updating positioning based on scroll events. In this case there is a <ytg-persistent-layer id="player" ...> which is position:fixed and they are updating the 'top' property in the scroll event.

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

Tracking for 46 and 47 as APZ is enabled in nightly and aurora. Sounds unclear if it's something we expect YouTube will fix or if we have to work around it.

	arni2033
	Comment 4 • 8 years ago

Unrelated comment.
On that site (https://gaming.youtube.com/watch?v=gCISR0T3grE) I noticed that when I autoscroll the left block (containing the video), there're very noticeable lags every second. No lags on Chrome.

Is it worth reporting? Probably it'll be fixed with APZ, but sometimes APZ just hides cases when Firefox overuses CPU/RAM, so this still may be useful.

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

It's probably worth reporting, yeah.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 6 • 8 years ago

Should we still be tracking this on our side for 46?

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

I don't think so. On Windows Edge has the same problem as we do (although it's a little less pronounced, I think because they paint faster).

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 8 • 8 years ago

This is evangelism, not tracking.

	u279076
	Comment 9 • 8 years ago

Kats, should this bug be moved to the Tech Evangelism?

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

Maybe. It's in the same category as the other apz-evangelism bugs [1], so if you want to move this one then they should all be moved. In some cases we need to do more work to figure out what alternative we should be recommending. (In this case, position:sticky should do it).

https://bugzil.la/whiteboard:apz-evangelism

	arni2033
	Comment 11 • 8 years ago

Last time I checked, it was fixed (as well as my bug 1247510, but it was rather about performance...)
@ Alexandra Lucinet (reporter):   Does that site look fixed to you?

	u279076
	Comment 12 • 8 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
> Maybe. It's in the same category as the other apz-evangelism bugs [1], so if
> you want to move this one then they should all be moved. 

I don't want to break how you're tracking this work so let's leave it be for now. Thanks.

	Ada [:adalucinet] Reporter
	Comment 13 • 8 years ago

(In reply to arni2033 from comment #11)
> Last time I checked, it was fixed (as well as my bug 1247510, but it was
> rather about performance...)
> @ Alexandra Lucinet (reporter):   Does that site look fixed to you?

Indeed this issue is no longer reproducible with latest Developer Edition nor Nightly, across platforms [1]. Marking accordingly.

[1] Ubuntu 14.04 32-bit, Windows 10 64-bit and Mac OS X 10.9.5

	Petruta Horea [:phorea], Desktop QA
	Comment 14 • 8 years ago

We retested this during APZ smoke test on latest Aurora 48.0a2.

The video is now jumpy during scrolling, but at a much lower scale and it doesn't exceed the video area. 
Also it overlaps the scrollbar in the right side by a few pixels. Re-opening.

Screenshot showing the above issues: http://i.imgur.com/ExM4R5H.png

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

Looks like the <ytg-persistent-player id="player" ...> element is position:fixed and has z-index:1, and they're intercepting mousewheel/DOMMouseScroll events and adjusting the position of the element manually. No idea why they're doing all this.

	Petruta Horea [:phorea], Desktop QA
	Comment 16 • 8 years ago

Disabling APZ per page doesn't improve scrolling behavior. I think this no longer depends on bug 1246290.

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

(In reply to Petruta Rasa [QA] [:petruta] from comment #16)
> Disabling APZ per page doesn't improve scrolling behavior. I think this no
> longer depends on bug 1246290.

This is a deficiency in our APZ-force-disabling functionality; I split it out into bug 1276361.

In this bug, let's focus on the evangelism issue. I'm moving the bug to the Tech Evangelism component, as I have been doing for other [apz-evangelism] bugs that I think are ready for attention by the evangelism team.

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

(In reply to Petruta Rasa [QA] [:petruta] from comment #14)
> We retested this during APZ smoke test on latest Aurora 48.0a2.
> 
> The video is now jumpy during scrolling, but at a much lower scale and it
> doesn't exceed the video area. 
> Also it overlaps the scrollbar in the right side by a few pixels. Re-opening.
> 
> Screenshot showing the above issues: http://i.imgur.com/ExM4R5H.png

Actually, I'm seeing the video lag during scrolling even with APZ disabled, it's just less pronounced that with APZ enabled.

Petruta, can you confirm that you see this too? That is, that APZ is just making worse an artifact that's already there without it?

	Petruta Horea [:phorea], Desktop QA
	Comment 19 • 8 years ago

That's right. I don't necessary observe a video lag, but the video is jumpy during scrolling also on Firefox 44.0.1 release.

At this point I think we can remove all the dependencies.

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

Thanks for verifying! As APZ does make the problem worse / more visible, I think it make sense to keep it as a dependency.

As for bug 1246290, I swapped it out for bug 1276361, which is what we'd need to be fixed for bug 1246290 to help.

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

This observation:

(In reply to Botond Ballo [:botond] from comment #18)
> Actually, I'm seeing the video lag during scrolling even with APZ disabled,
> it's just less pronounced that with APZ enabled.

[to be clear, by "lag" I mean that the video's position follows the scroll after a delay rather than immediately, not that the video playback is laggy]

is not consistent with this diagnosis:

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)
> Looks like the <ytg-persistent-player id="player" ...> element is
> position:fixed and has z-index:1, and they're intercepting
> mousewheel/DOMMouseScroll events and adjusting the position of the element
> manually. No idea why they're doing all this.

because with APZ disabled, the scroll position change and the video repositioning should get painted in the same refresh driver tick, and thus we should see no lag.

We need to investigate some more what is going here.

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

It looks like they are performing the repositioning from a requestAnimationFrame callback (https://gaming.youtube.com/s/gaming/907a71e7/gaming_polymer.js line 10027), which explains the non-APZ behaviour (requestAnimationFrame is asynchronous with respect to the scroll position change).

Interestingly, on Chrome there is no lag at all, suggesting that they're doing something different (better) from our non-APZ codepath. (IE11 has the lag as well, by the way. I don't have access to Edge or Safari; if someone would like to test those, please go ahead.)

So, there are two things to do:

  1) Investigate the difference between what Chrome is doing and our non-APZ codepath.
     This is unrelated to APZ.

  2) Consider detecting position changes from a requestAnimationCallback as scroll-linked
     effects, and disable APZ for them (with bug 1246290).
     I'll repurpose bug 1276361 for this.

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

(In reply to Botond Ballo [:botond] from comment #22)
>   1) Investigate the difference between what Chrome is doing and our non-APZ codepath.
>      This is unrelated to APZ.

Filed bug 1277327 for this.

	Mike Taylor [:miketaylr]
	Comment 24 • 8 years ago

Dennis, when you get a chance can you point out where they're updating pos:fixed according to Comment #2? At that point we can talk to Google and see if they're willing to move things to position:sticky.

	Mike Taylor [:miketaylr]
	Comment 25 • 8 years ago

(oops, now with actual ni?)

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

(In reply to Mike Taylor [:miketaylr] from comment #24)
> Dennis, when you get a chance can you point out where they're updating
> pos:fixed according to Comment #2?

I'm not Dennis but I think I've already answered this in comment 22: they are updating the "top" property of the position:fixed element ("ytg-persistent-player") in a requestAnimationFrame callback on https://gaming.youtube.com/s/gaming/907a71e7/gaming_polymer.js line 10027.

Let me know if that answers the question or if other details are needed.

	Mike Taylor [:miketaylr]
	Comment 27 • 8 years ago

Whoops, missed that comment -- thanks Botond!

Karl, over to you.

	Karl Dubost💡 :karlcow Assignee
	Comment 28 • 8 years ago

so position:sticky is a hard sell for Google. 
I'm waiting for their move on another bug about position: sticky, but the initial response was Why should we do it it's only supported in Firefox. 

So if someone gets the same answer, the list of reasons are outlined here.


1. https://www.chromestatus.com/features/6190250464378880
  and https://bugs.chromium.org/p/chromium/issues/detail?id=231752#c49
  If  I understand it's ready to go more public.

2. working in Safari and Firefox
  http://caniuse.com/#feat=css-sticky

3. Under consideration for Edge
  https://developer.microsoft.com/en-us/microsoft-edge/platform/status/positionsticky

4. (Google only properties) They already send most of the time different versions to Firefox and Chrome.


not clearing ni to remind me to take a look at that once the news.google.com is clearer.

Though here it would be YouTube team maybe it will be different. :)

	Karl Dubost💡 :karlcow Assignee
	Comment 29 • 8 years ago

Progress on news.google.com Bug 1256648#c13 but not yet a fix.

Let me try with YouTube. (different crowd).
Contacted through the partner mailing-list

	Karl Dubost💡 :karlcow Assignee
	Comment 30 • 8 years ago

on macosx with Firefox Nightly 52.0a1 (2016-09-25) (64-bit), I can't reproduce.
I sent a reminder on the list as it was not acknowledged.

	Andrew Overholt [:overholt]
	Comment 31 • 8 years ago

Fix-optional because this is a site issue.

	Mike Taylor [:miketaylr]
	Comment 32 • 6 years ago

(In reply to Karl Dubost :karlcow from comment #28)
> so position:sticky is a hard sell for Google. 
> I'm waiting for their move on another bug about position: sticky, but the
> initial response was Why should we do it it's only supported in Firefox. 

This still reproduces, and since it's been reported all major browsers are shipping position:sticky. Karl, can we get in touch with Google/Youtube again? Thanks.

	Karl Dubost💡 :karlcow Assignee
	Comment 33 • 6 years ago
contact email

Already 1.5 years have passed… pfew.
ok let's try again.

* Initial contact was on September 5, 2016 (no reply)
* Then September 27, 2016 (no reply)
* and today, Mars 12, 2018

All through the YouTube partner mailing list.

	Mike Taylor [:miketaylr]
	Comment 34 • 6 years ago

It looks like they actually replied 6 months ago (or cc'd someone from the right team).

I don't see the issue anymore, and the console doesn't complain about scroll-linked positioning effects. So let's call this FIXED.