1253446 - Camera indicator is in the wrong position

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Marco Castelluccio [:marco]]

Attachment: Schermata del 2016-03-03 23-03-01.png

Comment 1

[Marco Castelluccio [:marco]]

There are actually two regressions, bug 890156 moved it to a wrong position (but at least still visible), bug 1240749 moved it to the current wrong position.

Comment 2

[Jonathan Kew [:jfkthame]]

Hmm - what exactly is this "camera indicator"? I'm not familiar with it at all. Is this a Linux-specific UI element, or should I be seeing something similar on other platforms?

Comment 3

[Marco Castelluccio [:marco]]

It's the indicator that a camera is being used by a webpage. You can see it if you accept the getUserMedia prompt, e.g. on this page: http://mdn-samples.mozilla.org/s/webrtc-capturestill/.

Comment 4

[Jonathan Kew [:jfkthame]]

Yeah, I figured that's what it means, but I hadn't ever seen (noticed) it before. Just checked on OS X, and here we add an icon to the system menubar, so that must be a quite separate, system-specific implementation.

Just tried on Windows, too, and I see we do have the same indicator as Linux -- and a similar problem with its position. Though even Firefox 44 mispositioned it for me on Windows when I tried with a multi-monitor configuration, though not as badly as Nightly.

Do you happen to know what this thing is called in the Firefox code, or where it's implemented?

Comment 5

[Marco Castelluccio [:marco]]

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> Just tried on Windows, too, and I see we do have the same indicator as Linux
> -- and a similar problem with its position. Though even Firefox 44
> mispositioned it for me on Windows when I tried with a multi-monitor
> configuration, though not as badly as Nightly.

I'm currently using a single monitor, can you reproduce it if you use a single monitor?

> Do you happen to know what this thing is called in the Firefox code, or
> where it's implemented?

I'm not sure, maybe somewhere in this file: http://mxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI.jsm.

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Marco Castelluccio [:marco] from comment #5)
> (In reply to Jonathan Kew (:jfkthame) from comment #4)
> > Just tried on Windows, too, and I see we do have the same indicator as Linux
> > -- and a similar problem with its position. Though even Firefox 44
> > mispositioned it for me on Windows when I tried with a multi-monitor
> > configuration, though not as badly as Nightly.
> 
> I'm currently using a single monitor, can you reproduce it if you use a
> single monitor?

Yes, if the dpi scaling is > 100%. (With Nightly, that is; release Firefox handles it OK in this case.)

> 
> > Do you happen to know what this thing is called in the Firefox code, or
> > where it's implemented?
> 
> I'm not sure, maybe somewhere in this file:
> http://mxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI.jsm.

Thanks, that helps. Specifically, it looks like the code in browser/base/content/webrtcIndicator.js is responsible for positioning it; I'll see if I can figure out where the confusion is coming from.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: patch 1 - Convert from device to CSS pixels when positioning the webrtc indicator

The problem here is that the webrtcIndicator code doesn't convert between desktop pixels (from the nsIScreen APIs) and CSS pixels; and since landing per-monitor DPI support, these are no longer equivalent, as they used to be (on Linux and Windows). So a bit more care is needed in this code.

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: patch 2 - Return the proper scaling factor when querying the primary screen on Windows

On Windows, there's a secondary problem as well, which is that defaultCSSScaleFactor fails to return the correct value when called on primaryScreen. This is because for primaryScreen, the code just uses a null HMONITOR, but handing that to WinUtils::LogToPhysFactor doesn't work. With this fixed, I'm now seeing the webrtc indicator positioned correctly on Windows.

Comment 9

[Jonathan Kew [:jfkthame]]

Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed9a096ef848

Comment 10

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a06c1a146e16a369ab3648e184d9e3311362395
Bug 1253446 - patch 1 - Convert from device to CSS pixels when positioning the webrtc indicator. r=emk

https://hg.mozilla.org/integration/mozilla-inbound/rev/431e74ed28d9ebbbe01ed7278efa79b6dd8db5aa
Bug 1253446 - patch 2 - Return the proper scaling factor when querying the primary screen on Windows. r=emk

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2a06c1a146e1
https://hg.mozilla.org/mozilla-central/rev/431e74ed28d9