Closed Bug 1041685 Opened 6 years ago Closed 6 years ago

show hidpi icons in the mac global webrtc sharing indicator

Categories

(Firefox :: Site Permissions, defect)

x86
macOS
defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.2
Tracking Status
firefox33 --- verified
firefox34 --- verified

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(1 file, 1 obsolete file)

I couldn't do this while working in bug 1037430 because of bug 899334 which is now fixed.
Flags: firefox-backlog+
Blocks: 1016548
Hi Florian, can you mark this bug as either [qa+] or [qa-] for verification.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Iteration: --- → 34.1
QA Whiteboard: [qa?]
Flags: needinfo?(florian)
QA Whiteboard: [qa?] → [qa+]
Flags: needinfo?(florian)
Blocks: 1040061
QA Contact: drno
Whiteboard: [sceensharing-uplift]
Attached patch WIP - doesn't work (obsolete) — Splinter Review
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.
Attached patch Patch v2Splinter Review
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.
Attachment #8463400 - Attachment is obsolete: true
Attachment #8464800 - Flags: review?(dolske)
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.
Attachment #8464800 - Flags: review?(dolske) → review+
https://hg.mozilla.org/mozilla-central/rev/32fdcd1926fb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
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
Attachment #8464800 - Flags: approval-mozilla-aurora?
Iteration: 34.1 → 34.2
Attachment #8464800 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: drno → florin.mezei
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?
Flags: needinfo?(jbecerra)
QA Contact: florin.mezei → jbecerra
As this is part of screensharing I'd like Nils to verify this.
Flags: needinfo?(jbecerra) → needinfo?(drno)
QA Contact: jbecerra → drno
Not 100% sure what to look for exactly. But all three symbols (camera, microphone and screens) look ok on my retina display.
Flags: needinfo?(drno)
Verified with 33.0a2 (2014-08-12) and 34.0a1 (2014-08-12)
Marking as done.
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
Whiteboard: [sceensharing-uplift]
Component: General → Device Permissions
You need to log in before you can comment on or make changes to this bug.