Remove customize mode UI grid styling on window edges / behind the hamburger panel (and corresponding animations)

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Toolbars and Customization
P1
normal
VERIFIED FIXED
5 months ago
20 days ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks: 2 bugs)

53 Branch
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified)

Details

(Whiteboard: [photon-structure])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
See https://mozilla.invisionapp.com/share/5ZAEYEW8M#/screens/225203398

We'll get rid of the blue/grey background with a white grid and the padding animation (the latter will require some JS code changes, I believe).

Updated

4 months ago
Flags: qe-verify+
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon] → [photon-structure]

Comment 1

3 months ago
Mock-up has moved to here: https://mozilla.invisionapp.com/share/8HBR8ZD2B#/230606754_Customize_-_First_Run
(Assignee)

Updated

3 months ago
Whiteboard: [photon-structure] → [reserve-photon-structure]
(Assignee)

Updated

3 months ago
Priority: P2 → P3

Updated

2 months ago
Priority: P3 → P2
Whiteboard: [reserve-photon-structure] → [photon-structure]
(Assignee)

Updated

2 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 56.2 - Jul 10
Priority: P2 → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 months ago
This patch removes the transition and extra grid-styled padding/borders on the window, conditional on the MOZ_PHOTON_THEME ifdef which will track nightly until 57. As a result, the "CSS only" CART transition stuff gets omitted, and a cart run has 6 subtests (it has 9 right now). All the remaining tests are either a wash or substantial improvements.

I have 2 questions for jmaher:
- is the subtests disappearing going to break anything?
- is the fact that the .half subtests now log:

13:58:12     INFO -  PID 8864 | [#0] 1-customize-enter.half.TART  Cycles:25  Average:0.00  Median:0.00  stddev:0.00 (NaN%)  stddev-sans-first:0.00
13:58:12     INFO -  PID 8864 | Values: 0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0  0.0

Note in particular the stddev (NaN%) - going to break anything ?
Flags: needinfo?(jmaher)
(Assignee)

Comment 4

2 months ago
Patch is on top of bug 1354145, explicitly marking that dep.
Depends on: 1354145
removing a subtest is no big deal- we should see a change in the summarized number, just make myself and :igoldan aware when this lands and we will copy the change here and will acknowledge this as a test change.

regarding the .half tests, that is not good if we have 0.0- I assume this is the subtest being removed?
Flags: needinfo?(jmaher)
(Assignee)

Comment 6

2 months ago
(In reply to Joel Maher ( :jmaher) from comment #5)
> removing a subtest is no big deal- we should see a change in the summarized
> number, just make myself and :igoldan aware when this lands and we will copy
> the change here and will acknowledge this as a test change.
> 
> regarding the .half tests, that is not good if we have 0.0- I assume this is
> the subtest being removed?

Effectively, yes, I'm ifdef-ing out the things that would cause that subtest to measure something, I think. How bad "not good" is this going to be? As in, do I need to write up a patch to ignore these subtests or something, or can we live with it?
Flags: needinfo?(jmaher)
we can live with it, it will change the numbers slightly which is ok.  There are many other tests which report 0 values in a subtest.
Flags: needinfo?(jmaher)

Comment 8

2 months ago
mozreview-review
Comment on attachment 8881751 [details]
Bug 1354123 - remove customize mode transition,

https://reviewboard.mozilla.org/r/152850/#review158500

Looks good, I expect a nice CART improvement here.
Attachment #8881751 - Flags: review?(jaws) → review+

Comment 9

2 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/328777d96619
remove customize mode transition, r=jaws

Comment 10

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/328777d96619
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Some very noticeable improvements have landed! Thanks for the good work!

== Change summary for alert #7617 (as of June 29 2017 17:12 UTC) ==

Improvements:

 63%  cart summary osx-10-10 opt e10s     32.98 -> 12.05
 59%  cart summary linux64 opt e10s       21.09 -> 8.55
 59%  cart summary windows7-32 opt e10s   25.93 -> 10.59
 58%  cart summary linux64 pgo e10s       18.22 -> 7.65
 57%  cart summary windows10-64 opt e10s  21.98 -> 9.45
 57%  cart summary windows10-64 pgo e10s  18.02 -> 7.82
 56%  cart summary windows7-32 pgo e10s   18.66 -> 8.30

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7617

Comment 12

2 months ago
I have reproduced this Bug with Nightly 55.0a1 (2017-04-06) on Windows 10, 64 Bit!

The bug's fix is now verified on latest Nightly 56.0a1

Build ID 	20170706060058
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170705]
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
Flags: qe-verify+
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.