Dev Edition (compact dark/light) theme: solid grayscale circle appears instead of blue (i) icon in location bar for icons used by add-ons

VERIFIED FIXED in Firefox -esr52

Status

()

defect
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: 684sigma, Assigned: dao)

Tracking

({regression})

52 Branch
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 wontfix, firefox-esr52 verified, firefox53+ verified, firefox54 verified, firefox55 verified)

Details

Attachments

(3 attachments)

Reporter

Description

2 years ago
I have a problem with Firefox Beta 52. It doesn't happen in Firefox ESR 45.
Sometimes solid grayscale circle appears in location bar, while it's expected that blue (i) icon should appear.
It happens unpredictably and I don't remember all cases, however, I noticed one specific scenario when it happens

1. Install https://addons.mozilla.org/en-US/firefox/addon/devedition-theme-enabler/ , enable developer theme
2. Install https://addons.mozilla.org/en-US/firefox/addon/greasemonkey , restart the browser
3. Open http://example.org , click dropmarker in greasemonkey button, click "New user script", create a script


Result: solid grayscale circle appears in location bar
Expected: blue (i) icon should appear

DevEdition theme is my default install ever since curved tabs of Australis theme were implemented.

Updated

2 years ago
Component: Untriaged → Theme

Comment 1

2 years ago
Can you show a screenshot of what you're seeing? Does this happen in Firefox 53 (https://beta.mozilla.org/ ), where you no longer need an add-on to have the 'devedition' theme (now called 'compact light' and 'compact dark') ?
Flags: needinfo?(684sigma)
Reporter

Comment 2

2 years ago
Screenshot attached.
Yes, it does happen in Firefox Beta 53.
Flags: needinfo?(684sigma)

Comment 3

2 years ago
[Tracking Requested - why for this release]:
Regression in behavior of prominent add-ons

Range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bc9d199d895c33f1b4984afe61ec464a43a2a61a&tochange=c55bcb7c777ea09431b4d16903ed079ae5632648

This looks like bug 1304708 to me. Dão, any idea what's going on here?
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(dao+bmo)
Keywords: regression
Summary: Regression DevEdition theme: solid grayscale circle appears instead of blue (i) icon in location bar sometimes → Dev Edition (compact dark/light) theme: solid grayscale circle appears instead of blue (i) icon in location bar for icons used by add-ons

Updated

2 years ago
Blocks: 1304708

Updated

2 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 5

2 years ago
(In reply to :Gijs from comment #3)
> This looks like bug 1304708 to me. Dão, any idea what's going on here?

It's this change: https://hg.mozilla.org/mozilla-central/rev/a481cba45293#l4.12

I hadn't considered that add-ons might have multicolored icons.
Flags: needinfo?(dao+bmo)
For simpler STR, you can reproduce this in compact theme by entering 

PopupNotifications.show(gBrowser.selectedBrowser, "test", "hello", null, {callback: () => {}, label: "ok", accessKey: "o"}, [], {})

in the browser toolbox console.
(We probably also want to get rid of that old icon)
Assignee

Comment 8

2 years ago
Ah, so this isn't actually an add-on icon. In that case, since there were no other complaints, maybe we can leave the code as is and just replace this icon.
Yeah, I'm not even sure if it would be easy for add-ons to add an icon in the identity block. 

Realistically with the old extension API going away access to PopupNotifications would no longer be possible anyway. Bug 1331557 is tracking access for WebExtensions but I already mentioned that these should probably be anchored at the browser action icon of the extension instead.

So this icon, in whatever form, will likely not be publicly visible anymore after 57 if the deprecation timeline still stands. It might still be useful to have a fallback icon for internal usage I guess.
Assignee

Updated

2 years ago
Assignee: nobody → dao+bmo

Comment 11

2 years ago
mozreview-review
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

https://reviewboard.mozilla.org/r/119146/#review121496

Hm, the icon looks a bit different than the identity icon for me which is a bit irritating. I'll attach a screenshot. Probably need to fiddle with the size values.

I think I'd be fine with landing this if the small differences to the identity icon are resolved but I'm secretly hoping someone has a 16x16 SVG dialog icon lying around to replace this instead. Maybe there's a different icon we could re-use instead?

The proper way to solve this would probably be to get someone from UX to chime in, at the very least to say "this is almost invisible to users, so do whatever you like".

Philipp, do you have any feedback and/or ideas on how to solve this?
Attachment #8846060 - Flags: review?(jhofmann)
Flags: needinfo?(philipp)
Assignee

Comment 13

2 years ago
(In reply to Johann Hofmann [:johannh] from comment #11)
> I think I'd be fine with landing this if the small differences to the
> identity icon are resolved but I'm secretly hoping someone has a 16x16 SVG
> dialog icon lying around to replace this instead. Maybe there's a different
> icon we could re-use instead?

We have a few other info icons but they all come with their own differences.

> The proper way to solve this would probably be to get someone from UX to
> chime in, at the very least to say "this is almost invisible to users, so do
> whatever you like".

It think we've already established that this invisible to most users :)
Comment hidden (mozreview-request)

Comment 15

2 years ago
mozreview-review
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

https://reviewboard.mozilla.org/r/119146/#review121666

Comparing them again I think the new flat style isn't that far off the original. The old logo was also an (i) and yeah, this is only a fallback.

r=me

Thanks!
Attachment #8846060 - Flags: review?(jhofmann) → review+

Comment 16

2 years ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/22d5bcbc967a
Make default notification anchor icon be part of notification-icons.svg. r=johannh
Assignee

Updated

2 years ago
Flags: needinfo?(philipp)

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/22d5bcbc967a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee

Updated

2 years ago
Flags: qe-verify+
Track 53+ as regression.

Hi ::dao,
Since this also affects 53/53, do you think the patch is worth uplifting to 53/54?
Flags: needinfo?(dao+bmo)
Assignee

Comment 19

2 years ago
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1304708
[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]: STR in comment 6
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: it just replaces an old icon with a new one. pretty straightforward.
[String changes made/needed]: /
Flags: needinfo?(dao+bmo)
Attachment #8846060 - Flags: approval-mozilla-beta?
Attachment #8846060 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)

Comment 21

2 years ago
Build ID: 20170320030209
User AgentMozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

Verified as fixed on Firefox Nightly 55.0a1 on Windows 10 x 64, Mac OS X 10.11 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Flags: needinfo?(brindusa.tot)
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

Fix an UI regression that the icon is shown incorrectly in the location bar and was verified. Beta53+ & Aurora54+.
Attachment #8846060 - Flags: approval-mozilla-beta?
Attachment #8846060 - Flags: approval-mozilla-beta+
Attachment #8846060 - Flags: approval-mozilla-aurora?
Attachment #8846060 - Flags: approval-mozilla-aurora+
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

See comment 19. Grafts cleanly to ESR52.
Attachment #8846060 - Flags: approval-mozilla-esr52?
I can confirm this issue is fixed, I verified using Firefox 53.0b6, 54.0a2 on Win 8.1 x64 and Mac OS X 10.11 and Ubuntu 14.04 x64.
Comment on attachment 8846060 [details]
Bug 1345716 - Make default notification anchor icon be part of notification-icons.svg.

I guess this can't hurt.  UI regression fix, esr52+.
Attachment #8846060 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Timea, could you please take a look at this on 53b10 as well?
Flags: needinfo?(timea.zsoldos)
I can confirm this issue is fixed, I verified using Firefox 53.0b10 on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 14.04 x64.
I will re-verify this issue using Fx 52.1esr once the build is available.
Flags: needinfo?(timea.zsoldos)
I can confirm this issue is fixed, I verified using Firefox 52.1esr on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 14.04 x64.
You need to log in before you can comment on or make changes to this bug.