Rearranging an existing tab tile in about:newtab is sluggish

VERIFIED FIXED in Firefox 51



3 years ago
2 years ago


(Reporter: Fanolian+bugzilla, Assigned: dao)


({regression, reproducible})

53 Branch
Firefox 53
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox51+ verified, firefox52 verified, firefox53 verified)



(4 attachments)



3 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170111030235

Steps to reproduce:

1. In a new profile, make sure browser.newtabpage.compact is set to false (current default).
2. Open about:newtab.
3. Try to move an existing tab tile around.

Actual results:

(Please refer to attached video)
The animation is sluggish. It leaves a bad feedback for users.

Expected results:

The tile animation should follow mouse cursor closely.

Comment 1

3 years ago
From Mozregression:
Last good Nightly: 2017-01-04
First bad Nightly: 2017-01-05

Further bisection:

Regressed by: bug 1322738
Blocks: 1322738
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → New Tab Page

Comment 2

3 years ago
Animation was smooth before fixing bug 1322738.

Comment 3

3 years ago
If I set browser.newtabpage.compact to true, the animation is smooth but with a little bit stuttering. Probably it's just my computer.


3 years ago
Attachment #8825807 - Attachment description: about:newtab before fixing bug 1322738 → [GOOD] 2017-01-04 new tab.mp4

Comment 4

3 years ago
Dão, any idea what's going on here? The difference seems quite stark in the video.
Flags: needinfo?(dao+bmo)

Comment 5

3 years ago
Posted patch patchSplinter Review
Super-slow CSS selectors apparently.

This makes the hover effect for non-compact match compact. Not a big deal.
Assignee: nobody → dao+bmo
Ever confirmed: true
Flags: needinfo?(dao+bmo)
Attachment #8825823 - Flags: review?(gijskruitbosch+bugs)

Comment 6

3 years ago
Comment on attachment 8825823 [details] [diff] [review]

From IRC:

[15:06:59]	Gijs	dao: looking at your patch, why was the newtab-site selector originally changed to body:not(.compact) .newtab-site, and what effect are we now having on the new-style thumbnails that we weren't before?
[15:07:23]	Gijs	dao: like, I get that if we don't change the selector its specificity might override the now less-specific hover styles
[15:07:42]	Gijs	dao: but I'm confused whether this impacts the .compact styling at all because I don't see anything that overrides those properties for the .compact case

Clearing review for now.
Attachment #8825823 - Flags: review?(gijskruitbosch+bugs)

Comment 7

3 years ago
Comment on attachment 8825823 [details] [diff] [review]

Review of attachment 8825823 [details] [diff] [review]:

OK, per conversation on IRC and poking at this live against beta, I guess this works. We get a box shadow effect back in the compact case which in the grand scheme of things shouldn't matter.
Attachment #8825823 - Flags: review+

Comment 8

3 years ago
We should make sure this hits beta/aurora as well as we don't want to regress anything outside of the funnelcake.

Comment 9

3 years ago
Pushed by
Remove body:not(.compact) from selectors to undo dragging performance regression. r=gijs

Comment 10

3 years ago
[Tracking Requested - why for this release]: regression from bug 1322738

Comment 11

3 years ago
Comment on attachment 8825823 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: bug 1322738
[User impact if declined]: see comment 0
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: /
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: it only reverts part of bug 1322738
[String changes made/needed]: /
Attachment #8825823 - Flags: approval-mozilla-beta?
Attachment #8825823 - Flags: approval-mozilla-aurora?

Comment 12

3 years ago
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Track 51+ as regression in rearranging an existing tab tile in about:newtab
Comment on attachment 8825823 [details] [diff] [review]

Fix a UI regression about rearranging an existing tab tile. Beta51+ & Aurora52+. Should be in 51 beta 14.
Attachment #8825823 - Flags: approval-mozilla-beta?
Attachment #8825823 - Flags: approval-mozilla-beta+
Attachment #8825823 - Flags: approval-mozilla-aurora?
Attachment #8825823 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
Verified fixed on Windows 7 x64, Ubuntu 14.04 x64 and Mac OS X 10.11 using Firefox 51 RC (buildID: 20170116133120), latest Aurora 52.0a2 (buildID: 20170118004007) and latest Nightly 53.0a1 (buildID: 20170117030218).
You need to log in before you can comment on or make changes to this bug.