Fix snapshot list delete button style in Memory

VERIFIED FIXED in Firefox 57

Status

P3
normal
VERIFIED FIXED
a year ago
9 months ago

People

(Reporter: magicp.jp, Assigned: jdescottes)

Tracking

Trunk
Firefox 58

Firefox Tracking Flags

(firefox56 unaffected, firefox57 verified, firefox58 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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
(Reporter)

Updated

a year ago
Blocks: 1399028
status-firefox56: --- → unaffected
status-firefox57: --- → affected
(Assignee)

Comment 1

a year ago
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.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
(Assignee)

Updated

a year ago
Priority: -- → P3
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
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!
Attachment #8924115 - Flags: review?(gtatum) → review+

Comment 4

a year ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8f2173471a15
Fix styling of snapshot delete button in memory tool;r=gregtatum

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f2173471a15
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
(Reporter)

Comment 6

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

Comment 7

a year ago
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
Attachment #8924115 - Flags: approval-mozilla-beta?
Comment on attachment 8924115 [details]
Bug 1413478 - Fix styling of snapshot delete button in memory tool;

Recent regression, low risk, Beta57+
Attachment #8924115 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Reporter)

Comment 10

a year ago
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.
Flags: needinfo?(jdescottes)
(Assignee)

Comment 11

a year ago
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
Flags: needinfo?(jdescottes)
Attachment #8925467 - Flags: approval-mozilla-beta?
Comment on attachment 8925467 [details] [diff] [review]
Bug 1413478 - add context-fill to devtools close icon (beta-only)

extremely low risk, Beta57+
Attachment #8925467 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox57: fixed → affected
(Reporter)

Comment 14

a year ago
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!
Status: RESOLVED → VERIFIED
(Reporter)

Updated

a year ago
status-firefox57: fixed → verified
status-firefox58: fixed → verified

Updated

9 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.