Closed Bug 1391550 Opened 7 years ago Closed 7 years ago

chrome://browser/skin/notification-icons/geo-linux.svg is missing on Linux

Categories

(Firefox :: Theme, defect)

Product:
Firefox
Component:
Theme
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME
Milestone:
Firefox 57
Tracking Flags:
Tracking Status
firefox56 --- unaffected
firefox57 --- verified

People

(Reporter: johannh, Assigned: johannh)

References

Details

Attachments

(2 files)

Screen Shot 2017-08-17 at 18.44.54.png
109.01 KB, image/png
Details
Bug 1391550 - Move geo notification icons to their respective theme subfolders.
59 bytes, text/x-review-board-request
dao
: review+
Details 
I'm not sure why the patches in bug 1371230 didn't get backed out because of this (other people are getting failures, see https://bugzilla.mozilla.org/show_bug.cgi?id=1390499#c12), but the geo-linux.svg icon doesn't exist/work on Linux. See screenshot.

We should fix that :)
Flags: qe-verify+
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
We should move these away from browser/themes/shared/jar.inc.mn, into the platform-specific manifests, and get rid of the -linux, -windows, and -osx file name suffixes.

(Or better yet, use the same icon across platforms...)
(In reply to Dão Gottwald [::dao] from comment #3)
> We should move these away from browser/themes/shared/jar.inc.mn, into the
> platform-specific manifests, and get rid of the -linux, -windows, and -osx
> file name suffixes.

Yes. I agree.

> (Or better yet, use the same icon across platforms...)

I think we asked this a couple of times but UX seems quite sure they want to use different icons (which I can understand).
Have only tried this on OSX so far, might update the request after trying to compile on other platforms...
Comment on attachment 8898655 [details]
Bug 1391550 - Move geo notification icons to their respective theme subfolders.

https://reviewboard.mozilla.org/r/170028/#review175268

::: browser/themes/shared/notification-icons.inc.css:61
(Diff revision 2)
> -%endif
>  }
>  
>  .popup-notification-icon[popupid="geolocation"] {
>  %ifdef XP_MACOSX
> -  list-style-image: url(chrome://browser/skin/notification-icons/geo-osx.svg);
> +  list-style-image: url(chrome://browser/skin/notification-icons/geo.svg);

You can just add an override for geo-detailed.svg on Mac that points to geo.svg without packaging the icon twice: http://searchfox.org/mozilla-central/rev/e8c36327cd8c9432c69e5e1383156a74330f11f2/browser/themes/osx/jar.mn#115
Attachment #8898655 - Flags: review?(dao+bmo) → review+
Comment on attachment 8898655 [details]
Bug 1391550 - Move geo notification icons to their respective theme subfolders.

https://reviewboard.mozilla.org/r/170028/#review175300

::: browser/themes/osx/jar.mn:125
(Diff revisions 2 - 3)
>  % override chrome://browser/skin/menuPanel-customize@2x.png                chrome://browser/skin/yosemite/menuPanel-customize@2x.png               os=Darwin osversion>=10.10
>  % override chrome://browser/skin/menuPanel-exit.png                        chrome://browser/skin/yosemite/menuPanel-exit.png                       os=Darwin osversion>=10.10
>  % override chrome://browser/skin/menuPanel-exit@2x.png                     chrome://browser/skin/yosemite/menuPanel-exit@2x.png                    os=Darwin osversion>=10.10
>  % override chrome://browser/skin/menuPanel-help.png                        chrome://browser/skin/yosemite/menuPanel-help.png                       os=Darwin osversion>=10.10
>  % override chrome://browser/skin/menuPanel-help@2x.png                     chrome://browser/skin/yosemite/menuPanel-help@2x.png                    os=Darwin osversion>=10.10
> +% override chrome://browser/skin/notification-icons/geo-detailed.svg       chrome://browser/skin/notification-icons/geo.svg

Please move this further up, not in the middle of the 10.10 overrides. Might be a good idea to add blank lines around these blocks.
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/46a61cf0cfe0
Move geo notification icons to their respective theme subfolders. r=dao
https://hg.mozilla.org/mozilla-central/rev/46a61cf0cfe0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0

I couldn't reproduce the issue on Ubuntu 16.04 x64 with Nightly 57.0a1 (Build ID: 20170817100132) using the https://permission.site.

@Johann, on what Linux version did you encounter the bug?
Flags: needinfo?(jhofmann)
(In reply to roxana.leitan@softvision.ro from comment #14)
> Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
> 
> I couldn't reproduce the issue on Ubuntu 16.04 x64 with Nightly 57.0a1
> (Build ID: 20170817100132) using the https://permission.site.
> 
> @Johann, on what Linux version did you encounter the bug?

Ubuntu in the Nightly from 2017-08-17.
Flags: needinfo?(jhofmann)
Johann, can you still reproduce this issue by using the affected build (2017-08-17)?
I tried to reproduce it on Firefox 57.0a1(2017-08-17), under Ubuntu 16.04x64 and under Windows 10x64, without any luck.  
I also noticed that Roxana was not able to reproduce it as well. 
If you are able to reproduce the issue, please attached the STR in order for us to be able to confirm the fix.
Flags: needinfo?(jhofmann)
(In reply to Mihai Boldan, QA [:mboldan] from comment #16)
> Johann, can you still reproduce this issue by using the affected build
> (2017-08-17)?
> I tried to reproduce it on Firefox 57.0a1(2017-08-17), under Ubuntu 16.04x64
> and under Windows 10x64, without any luck.  
> I also noticed that Roxana was not able to reproduce it as well. 
> If you are able to reproduce the issue, please attached the STR in order for
> us to be able to confirm the fix.

It was fixed on the same day I think, maybe it's not in the build. I'm not sure it's important anymore whether we can reproduce this in an old build, to be honest. It's definitely fixed.
Flags: needinfo?(jhofmann)
Note that I was not able to reproduce this issue on affected builds, under Ubuntu 14.04x64.
The issue is also no reproducible on Firefox 57.0b14, under Windwos 7x86, Ubuntu 14.04x64 and under macOS 10.13.
Taking in consideration Comment 17 and my investigation, I'm marking this issue as RESOLVED-WORKSFORME.
status-firefox57: fixed → verified
Flags: qe-verify+
Resolution: FIXED → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: