1547326 - Storage inspector add/refresh button icons aren't visible

Status: RESOLVED FIXED

(DevTools :: Storage Inspector, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: missing-icons.png

STR:
Open DevTools -> Storage Inspector

Expected:

• "add" and "refresh" icons are visible on the buttons in the top bar

Actual:

• The buttons are visible, but the icons aren't

These are the buttons in the markup: https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/devtools/client/storage/index.xul#50-53.

And they are styled with CSS and ::before: https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/devtools/client/themes/storage.css#73-82.

Tentatively marking this as regressed by Bug 1538983 since it seems the most likely culprit.

Comment 1

[Brian Grinstead [:bgrins]]

It's because this rule doesn't match anymore .devtools-button:not(:empty) since the content isn't anonymous anymore: https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/devtools/client/themes/common.css#377 / https://searchfox.org/mozilla-central/search?q=.devtools-button%3Anot%28%3Aempty%29&path=.

I wonder if we still need to support non-empty .devtools-button elements anymore. Belén, do you know?

I think these are the only <xul:button> devtools-button elements so perhaps we can directly switch these to html:button which would also fix it.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1547326 - Convert storage inspector buttons to html:button in order to drop support for <xul:button class="devtools-button">

Comment 3

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #1)

It's because this rule doesn't match anymore .devtools-button:not(:empty) since the content isn't anonymous anymore: https://searchfox.org/mozilla-central/rev/444ee13e14fe30451651c0f62b3979c76766ada4/devtools/client/themes/common.css#377 / https://searchfox.org/mozilla-central/search?q=.devtools-button%3Anot%28%3Aempty%29&path=.

I wonder if we still need to support non-empty .devtools-button elements anymore. Belén, do you know?

Yeah, here's a list of places where we use them:

• netmonitor filter buttons
• console filter buttons
• perf tool empty notice
• memory tool empty notice

I think it would make sense to probably split .devtools-button into .devtools-text-button and .devtools-icon-button

Comment 4

[Tim Nguyen :ntim]

We also have .devtools-toolbarbutton which is defined for XUL toolbarbuttons.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f41ea9d40a05
Convert storage inspector buttons to html:button in order to drop support for <xul:button class="devtools-button"> r=jdescottes

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f41ea9d40a05

Comment 7

[Belén [:ladybenko]]

(sorry :bgrins, was on PTO when this was requested and forgot to update my status on bugzilla)