Closed Bug 1354943 Opened 7 years ago Closed 7 years ago

Overpainting on Youtube channel page when scrolling down

Categories

(Core :: Web Painting, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Performance Impact high
Tracking Status
platform-rel --- +

People

(Reporter: ehsan.akhgari, Unassigned)

Details

(Whiteboard: [platform-rel-Google][platform-rel-Youtube])

STR:

1. Go to https://www.youtube.com/channel/UCKcC11ircL-HI7TieCr9XQg
2. Scroll down.

When the "AutoExpertTV Videos Playlists Channels Discussion About" snaps into its fixed vertical position, we repaint the entire scrollable area containing the video thumbnails which causes a visible scroll jank.

As far as I can tell, nothing in the DOM of that area is changing.  What happens is that as the page scrolls down, at some point, the div with id masthead-appbar gets its class set from "hid" to "" and its style set from "display: none" to "" (which causes the visible title bar snapping mentioned above) but no other change happens in the DOM, specifically, the div with id channel-subheader which includes the scrolling area of the page which includes the "Home Videos Playlists Channels Discussion About" section (the scrolling one) stays the same, so ideally we should not be repaining this large layer.
Matt, any chance you could have a look, please?  Thanks!
Flags: needinfo?(matt.woodrow)
platform-rel: --- → +
Whiteboard: [qf:p1] → [qf:p1][platform-rel-Google][platform-rel-Youtube]
It looks like the repaint is caused by a div called "masthead-positioner-height-offset", at the very top of the "body-container", which gets a different height when the "appbar-hidden" class is toggled on and off for the body.

Removing this node prevents those excessive repaints, though of course then the content slides under the masthead.

I don't know enough about the layout and rendering to say whether this can or should be prevented.
I can reproduce this 'fix'.

Changing the height of an element near the top is going to move all the page content, so it's somewhat expected that we end up repainting everything. It's possible we could do some clever hacks to change our definition of the origin and paint things from the changed element up (instead of painting everything below it), but it's not a simple fix.

It doesn't seem like this has any noticeable effect on performance though, and even without the change the full repaint only has a raster time of 18ms (so we'd only drop a single frame).

We still completely stop scrolling when this happens, I suspect this has to do with the page change interfering with smooth scrolling, more than being a performance problem.
Flags: needinfo?(matt.woodrow)
I'm seeing different HTML served to Firefox vs. Chrome for this URL, so it's hard to tell if we're better or worse. Hard to measure YouTube perf relative to Chrome when the page layout is so different for the two browsers.
(This bug had gotten stuck deep down in the list of bugs I hadn't gotten around to look at yet, sorry about that.)

(In reply to Adam Gashlin [:agashlin] from comment #2)
> It looks like the repaint is caused by a div called
> "masthead-positioner-height-offset", at the very top of the
> "body-container", which gets a different height when the "appbar-hidden"
> class is toggled on and off for the body.
> 
> Removing this node prevents those excessive repaints, though of course then
> the content slides under the masthead.

From this description it seems that if we report this to YouTube they could potentially fix it, but regardless I just tested this on the new YouTube and the problem doesn't exist there.  I suggest closing this as WONTFIX because of that.
Hearing no objections, I'm WONTFIXing.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Performance Impact: --- → P1
Whiteboard: [qf:p1][platform-rel-Google][platform-rel-Youtube] → [platform-rel-Google][platform-rel-Youtube]
You need to log in before you can comment on or make changes to this bug.