Closed Bug 1216902 Opened 9 years ago Closed 9 years ago

On linux, chrome://mozapps/skin/passwordmgr/key.png is overridden despite a key.png file being there

Categories

(Toolkit :: Themes, defect)

Product:
Toolkit
Component:
Themes
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: glandium, Unassigned)

References

Details

Because the linux theme includes things from the windows theme, key.png is installed from the windows jar.mn. But the linux jar.mn has an override that makes key.png overridden with chrome://mozapps/skin/passwordmgr/key-16.png.

Now, the interesting thing is that key.png and key-16.png are not the same icon (and the windows key-16.png is also different from both).

It also turns out that before bug 706103, key.png /was/ a copy of key-16.png, and was replaced in bug 706103 by the override. But since things kind of relied on the files from the linux theme overwriting the files from the windows theme, the fact that the windows theme also had a key.png was simply unknown.

bug 1210703 is more examples of that kind of issues, and this kind of could be fixed there.
Is the icon actually wrong? As in, what's wrong with the current setup? Just that we're packaging files that we then never use, on Linux? Or something else?
Flags: needinfo?(mh+mozilla)
I'm still not sure I understand what issue you're flagging up.

Currently, on Linux, from what you're saying it sounds like we're using https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/linux/mozapps/passwordmgr/key-16.png for key.png, is that right? What's wrong with that?


Here's my understanding of what's going on:

The 64 versions are all used inside the login/pw doorhanger. It isn't surprising they are different from the tiny unsuffixed and -16 versions, so let's leave those aside for the moment.

So it turns out key.png is only used by this code:

https://dxr.mozilla.org/mozilla-central/rev/daa7d98525e859d32a3b3e97101e129a897192a1/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#746

That only runs if the LoginManagerPrompter doesn't have a PopupNotifications object, in which case we show a notification bar instead of a doorhanger. AIUI windows *always* have a PopupNotifications object in Firefox, which means the notification bar code is unused in Firefox (but potentially used by SeaMonkey, Thunderbird, or other consumers).

key-16.png is used by the current doorhanger code in use in Firefox.

To me the differences between the individual key.png files seem relevant - they have to do with the background of the notification bar on which the key.png files appear - the OS X one will be dark, which is why the icon is white and has no grey border - it will look like a relief against the dark background.

key-16.png on the other hand, will appear in the notification area inside the URL bar, on a white background.

We could potentially refactor this so that the icons had more descriptive names in terms of what they're currently used for. Would that make things better? I'm really still struggling with what the problem is. What's the desired fix? Should we make sure we don't package the Windows key.png on Linux, or something? The override will work as-is, I'm pretty sure... It doesn't seem to me like anything is actually broken.
Flags: needinfo?(mh+mozilla)
What this report is really about is key.png being shipped but unused because of the override. The rest of the discussion can be seen as a digression from the observation of the various files and what they look like, which may or may not have significance as to whether maybe we'd want to do something else than not shipping key.png.
Flags: needinfo?(mh+mozilla)
It seems like the sensible solution for this and bug 1210703 would be to create a separate jar.mn file that both the Windows and Linux files use/include/whatever, that only has items that aren't overridden or packaged separately by Linux. I don't know how many things would be in such a manifest (it is the opposite of the list in bug 1210703), but there we are...
Depends on: 1210703
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.