Closed
Bug 1368070
Opened 8 years ago
Closed 8 years ago
Twitter's "position:relative" body tweak causes jank due to frame reconstruction
Categories
(Web Compatibility :: Site Reports, enhancement)
Web Compatibility
Site Reports
Tracking
(platform-rel ?)
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| platform-rel | --- | ? |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
(Whiteboard: [platform-rel-Twitter][sitewait])
Attachments
(2 files)
I'm going to reach out to Twitter to ask that they make a change to their feed styling.
Right now, when you click a tweet to make it appear as a modal overlay, Twitter activates a CSS class "overlay-enabled" on the <body> which happens to set the body to "position:relative".
Unfortunately: changes to the CSS "position" property trigger a potentially-expensive path in Firefox -- we rebuild the box tree for the subtree rooted at the affected element, which in this case means the whole page, because the affected element is the <body>.
Fortunately, though: this "position: relative" tweak seems to be completely unnecessary!! At least, I can suppress it in devtools, and there doesn't seem to be any change in behavior.
So I'd like to suggest that Twitter avoid making this change. They could do this either by:
(A) Adding a more specific rule that gently overrides the problematic one:
body.overlay-enabled { position: static }
(B) Adding a rule that aggressively overrides the problematic one:
body { position: static !important }
(C) Making the body *always* position:relative, e.g.
body { position: relative }
With that change combined with the fix for bug 1367568 (which should be in Firefox Nightly tomorrow), twitter overlays should appear much faster on heavy twitter loads (with lots of backscroll), as noted in bug 1342220 comment 22.
|
Assignee | |
Comment 1•8 years ago
|
||
(In reply to Daniel Holbert [:dholbert] (reduced availability - travel & post-PTO backlog) from comment #0)
> Fortunately, though: this "position: relative" tweak seems to be completely
> unnecessary!! At least, I can suppress it in devtools, and there doesn't
> seem to be any change in behavior.
(I suspect the "position:relative" styling here is useful & important when the overlay-enabled element is some arbitrary div -- it lets abspos descendants be positioned with respect to that div. However, it's completely unnecessary when the overlay-enabled element is the body, because abspos descendants will already be positioned with respect to the body/viewport, so there's no behavior-change that Twitter's gaining by changing the body between position:static & position:relative.)
|
|
Assignee | |
Comment 2•8 years ago
|
||
I reached out to some folks at Twitter about this today, and they've got someone looking at it. Hooray!
|
|
Assignee | |
Comment 3•8 years ago
|
||
Steps to reproduce the issue, for testing/demonstration purposes:
0. Be logged in to Twitter (this is required to get much scrollback)
1. Visit https://twitter.com/CodingExon/with_replies
2. Press PageDown key until you see tweets from December 2014 (this takes a minute or so). Basically, you just want a lot of backscroll.
3. Scroll back to the top of the page (e.g. "Home" key)
4. Click a tweet.
I'm attaching a screencast of the issue (starting from STR step 2, having scrolled back to December 2014). The whole time that "May 30" is underlined (from 0:03 to 0:08), the page is hanging & unresponsive to user input.
|
|
Assignee | |
Comment 4•8 years ago
|
||
And for comparison, here's a screencast of what this looks like after I've added my first suggested workaround, in Firefox Nightly.
Still a little bit of jank, but much less!
(Note that the workaorunds only help in version 55 [current Nightly] because we need bug 1367568 & bug 1344398's fixes in order to completely avoid frame reconstruction here. If you test in Firefox release 53 or beta 54, you won't see a performance difference.)
|
||
Updated•8 years ago
|
platform-rel: --- → ?
Whiteboard: [platform-rel-Twitter][sitewait]
|
|
Assignee | |
Comment 5•8 years ago
|
||
Twitter has now pushed a fix for this! They went with option (C) -- making the body *always* position:relative -- which I verified using devtools.
I just performed the STR from comment 3, and I don't see any noticeable delay anymore -- after I click, the white box appears (as the beginnings of the modal tweet popup) pretty much right away.
So we can call this outreach bug FIXED. Thanks, Twitter!
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Comment 6•8 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #5)
> I just performed the STR from comment 3, and I don't see any noticeable
> delay anymore
Er -- actually, I was cheating slightly -- I had Stylo enabled in this profile. But that's good news -- in a default profile, we are now not-great-but-still-better-than-we-were, after Twitter's tweak. And with Stylo enabled, we're way better!
Details:
* In a fresh profile, this bug's STR produce a ~1 second delay (as discussed in bug 1342220 comment 22), almost entirely in restyling, with very little time spent in reflow. This is what we expected (and is an improvement from ~4 second delays that we had previously for these STR) -- it has to do with how Gecko handles restyles on the root of deep subtrees.
* With Stylo enabled, that remaining ~1 second delay basically disappears! (This is what I saw in comment 5.)
|
||
Updated•7 years ago
|
Product: Tech Evangelism → Web Compatibility
You need to log in
before you can comment on or make changes to this bug.
Description
•