Closed Bug 1330303 Opened 5 years ago Closed 5 years ago
Rearranging an existing tab tile in about:newtab is sluggish
766.63 KB, video/mp4
733.62 KB, video/mp4
559.45 KB, video/mp4
1.17 KB, patch
|Details | Diff | Splinter Review|
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.
From Mozregression: Last good Nightly: 2017-01-04 First bad Nightly: 2017-01-05 Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=57ac9f63fc6953f4efeb0cc84a60192d3721251f&tochange=f13abb8ba9f366c9f32a3146245adf642528becd Further bisection: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f897399fb28a18b34f5200677e2d585e5d4342a2&tochange=c8883a87c6dc7c8d355fc93c9ee08bb75cfbf7db Regressed by: bug 1322738
Animation was smooth before fixing bug 1322738.
If I set browser.newtabpage.compact to true, the animation is smooth but with a little bit stuttering. Probably it's just my computer.
Attachment #8825807 - Attachment description: about:newtab before fixing bug 1322738 → [GOOD] 2017-01-04 new tab.mp4
Dão, any idea what's going on here? The difference seems quite stark in the video.
Super-slow CSS selectors apparently. This makes the hover effect for non-compact match compact. Not a big deal.
Assignee: nobody → dao+bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8825823 [details] [diff] [review] patch 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.
Comment on attachment 8825823 [details] [diff] [review] patch 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+
We should make sure this hits beta/aurora as well as we don't want to regress anything outside of the funnelcake.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3ac85b75fea8 Remove body:not(.compact) from selectors to undo dragging performance regression. r=gijs
[Tracking Requested - why for this release]: regression from bug 1322738
Comment on attachment 8825823 [details] [diff] [review] patch 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]: /
Track 51+ as regression in rearranging an existing tab tile in about:newtab
Comment on attachment 8825823 [details] [diff] [review] patch Fix a UI regression about rearranging an existing tab tile. Beta51+ & Aurora52+. Should be in 51 beta 14.
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.