1413478 - Fix snapshot list delete button style in Memory

Status: VERIFIED FIXED

(DevTools :: Memory, defect, P3)

Description

[magicp]

Steps to reproduce:
1. Start Firefox 57b or Nightly
2. Open Developer Tools Memory
3. Take snapshot
4. Confirm delete button of snapshot list

Actual Results:
light-theme: hover style is too bright.
dark-theme: delete image is dark and hover style is different with others.

Expected Results:
Same with Firefox 56

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5291605468b85f824823b167b8a8006c87afe0f2&tochange=c5aedf62e0af308f076dd65d6775a566dea6365a

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for logging. 
I think we should make the styling of this button consistent with what we do in the style editor for the "eye" icon.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1413478 - Fix styling of snapshot delete button in memory tool;

Review commit: https://reviewboard.mozilla.org/r/195348/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195348/

Comment 3

[Greg Tatum [:gregtatum]]

Comment on attachment 8924115 [details]
Bug 1413478 - Fix styling of snapshot delete button in memory tool;

https://reviewboard.mozilla.org/r/195348/#review200682

Thanks for the fix!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f2173471a15
Fix styling of snapshot delete button in memory tool;r=gregtatum

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8f2173471a15

Comment 6

[magicp]

This bug fix has verified in the latest Nightly build (20171102100041). Please uplift 57.

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8924115 [details]
Bug 1413478 - Fix styling of snapshot delete button in memory tool;

Approval Request Comment
[Feature/Bug causing the regression]: 1399028 
[User impact if declined]: a delete icon in the DevTools memory panel is not visible
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple css change
[String changes made/needed]: none

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8924115 [details]
Bug 1413478 - Fix styling of snapshot delete button in memory tool;

Recent regression, low risk, Beta57+

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/84fdf0a64657 (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/806353d38dc4

Comment 10

[magicp]

I have confirmed this in 57.0b14(Build ID 20171102181127). Unfortunately, delete icon color is not white in selected list. In my guess, close.svg is different with Nightly.

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1413478 - add context-fill to devtools close icon (beta-only)

Sorry :(

I thought that Bug 1399886 had landed in 57 but it was only in 58. For this fix to work in beta we need an additional patch. It would really be nice to uplift this beta-only fix if there's still a little time. I built and tested on beta, with this additional fix the icon is always visible.

Sorry about that again.

Approval Request Comment
[Feature/Bug causing the regression]: 1399028 
[User impact if declined]: a delete icon in the DevTools memory panel is not visible
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no (beta only)
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: adding an optional "context-fill" keyword for one of our svg icons  
[String changes made/needed]: none

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8925467 [details] [diff] [review]
Bug 1413478 - add context-fill to devtools close icon (beta-only)

extremely low risk, Beta57+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/65e3f6bbe4f5 (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/36c015b08565

Comment 14

[magicp]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 ID:20171106194249
This bug fix has verified in Firefox 57. Thanks!