Florian Quèze [:florian] Assignee
	Description • 10 years ago

I couldn't do this while working in bug 1037430 because of bug 899334 which is now fixed.

	Marco Mucci [:MarcoM]
	Comment 1 • 10 years ago

Hi Florian, can you mark this bug as either [qa+] or [qa-] for verification.

	Florian Quèze [:florian] Assignee
	Comment 2 • 10 years ago

Based on bug 1037433 comment 17 and on attachment 8456098 [details] [diff] [review], this is how I think the patch should look like.

But this doesn't work at all: (blank/empty) icons are displayed in the menubar. Will need to debug.

	Florian Quèze [:florian] Assignee
	Comment 3 • 10 years ago

This time it actually works. The problem I had before was that the menu XUL node needs to be actually inserted in the document's DOM for the CSS to be applied. Should have been obvious :-/.

This patch uses the icons from attachment 8464749 [details] (from bug 1037418). We tweaked them a few times with Philipp to have a reasonable size and alignment on both retina and non-retina screens.

	Justin Dolske [:Dolske]
	Comment 4 • 10 years ago

Comment on attachment 8464800 [details] [diff] [review]
Patch v2

Review of attachment 8464800 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/hiddenWindow.xul
@@ +7,4 @@
>  
>  #ifdef XP_MACOSX
>  <?xul-overlay href="chrome://browser/content/macBrowserOverlay.xul"?>
> +<?xml-stylesheet href="chrome://browser/skin/webRTC-indicator.css" type="text/css"?>

Part of me wants to call this "menubar-indicator", since it's more generic and could be used by other things. The other part of me doesn't care, given that it's taken us this long to find a need for it. :) We can always rename it later should the need arise.

	Florian Quèze [:florian] Assignee
	Comment 5 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/32fdcd1926fb

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/32fdcd1926fb

	Florian Quèze [:florian] Assignee
	Comment 7 • 10 years ago

Comment on attachment 8464800 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: bug 1037430
[User impact if declined]: blurry icons for Mac users with the menubar placed on a retina screen.
[Describe test coverage new/current, TBPL]: currently in m-c, will be verified by QA.
[Risks and why]: low
[String/UUID change made/needed]: none

	Florian Quèze [:florian] Assignee
	Comment 8 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/306a60a1b803

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 9 • 10 years ago

We have no Mac with Retina that we can use to verify this. As it seems that Nils will not handle this, Juan can you take care of it instead?

	u279076
	Comment 10 • 10 years ago

As this is part of screensharing I'd like Nils to verify this.

	Nils Ohlmeier [:drno]
	Comment 11 • 10 years ago

Not 100% sure what to look for exactly. But all three symbols (camera, microphone and screens) look ok on my retina display.

	Nils Ohlmeier [:drno]
	Comment 12 • 10 years ago

Verified with 33.0a2 (2014-08-12) and 34.0a1 (2014-08-12)

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 13 • 10 years ago

Marking as done.