Share screen doesn't work on Mac with getUserMedia Test Page with multiple monitors

VERIFIED FIXED in Firefox 62

Status

()

P2
major
Rank:
15
VERIFIED FIXED
3 months ago
2 months ago

People

(Reporter: Ovidiu, Assigned: dminor)

Tracking

({regression})

Trunk
mozilla64
Unspecified
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox61 unaffected, firefox62 verified, firefox63 verified, firefox64 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
[Affected versions]:

Nightly 63.0a1
Beta 62

[Affected platforms]:

macOS 10.13

Prerequisites: You need to have 2 monitors

[Steps to reproduce]:

1. Open Firefox
2. Go to https://mozilla.github.io/webrtc-landing/gum_test.html
3. Select "Screen"
4. From the "Screen to share" drop-down select Screen 1 

[Expected result]:
The image from the Screen 1 is captured. 

[Actual result]:
The image from both screens is captured.


Note: This issue is a regression, I run mozregression and here is the push log: 


ast good revision: 89b72c9c5db5dc38c0dd6547eac9b1b1a777c53f
29:20.48 INFO: First bad revision: 0708df4d3b7a8c38dd32f52d3199e5a27a0758f2
29:20.48 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=89b72c9c5db5dc38c0dd6547eac9b1b1a777c53f&tochange=0708df4d3b7a8c38dd32f52d3199e5a27a0758f2

From what I see I think the fix from bug 1409018 created this regression. 
Also when I see the 2 screens the capture is not updated only from on 1 screen.
(Reporter)

Updated

3 months ago
status-firefox61: --- → unaffected
status-firefox62: --- → affected
Keywords: regression
(Reporter)

Comment 1

3 months ago
Dan please take a look at this and tell me your opinion.
Flags: needinfo?(dminor)
(Assignee)

Comment 2

3 months ago
Thank you for your report!

I tested this just now, and I'm seeing the following:
* with no screen attached, the screen share works and updates normally
* with an external screen attached and the display mirrored, screen share works and updates normally
* with an external screen attached an no mirroring, regardless of whether I pick Screen 1 or Screen 2 in the drop down, I always get the laptop screen (Screen 1), not the external screen. Sharing that screen works and updates normally.

For reference, I tested using a 2015 macbook with a retina display, running 10.13.1, and I used my tv as the second monitor.

I'm adjusting the bug title as the problem only seems to occur with multiple monitors attached. Please let me know if you are also seeing problems with just a single screen.

Since I'm not getting the exact same problem you are seeing, I was wondering if you could tell me a bit more about your test system. By two monitors, do you mean a laptop screen and a monitor, or two monitors connected to a desktop, or something else? How do you have your two monitors configured in System Settings? What are the resolutions of your two monitors? Thanks!
Assignee: nobody → dminor
Rank: 15
Flags: needinfo?(dminor) → needinfo?(ovidiu.boca)
Priority: -- → P2
Summary: Share screen doesn't work on Mac with getUserMedia Test Page → Share screen doesn't work on Mac with getUserMedia Test Page with multiple monitors
(Reporter)

Comment 3

3 months ago
I did a screen recording of the issue, https://streamable.com/fffm6
I have an iMac (21.5-inch, Late 2013) with an extra monitor S22C300 Display (Samsung), my Graphic card is Intel Iris Pro 1536 MB. The resolution of the monitors: iMac - 1920x1080   Samsung monitor 1080p, this is the arrangement of the monitors: https://imgur.com/a/6gA1t5u
From the screen recording, you can see that both monitors are displayed even if I select screen 1 or screen2. 
Please tell me if there is something else that I can do.
Flags: needinfo?(ovidiu.boca)
(Reporter)

Comment 4

3 months ago
I also tested this on a MacBook Pro Mac OS X 10.13 (13-inch,2017, Two Thunderbolt 3 ports) with an extra monitor Samsung S22C300 (HDMI cable) and the arrangement of the screens is this one: https://imgur.com/a/DL7eSU5 . and it doesn't matter what screen I choose from the drop down (1 or 2) the sharing is performed only for Samsung screen.
dan - can you try again?  Particularly the cabling setup, or something close.  This is shipping in 62 and isn't a stop-ship, but we should try to fix and uplift to 63 at least.
Flags: needinfo?(dminor)
(Assignee)

Comment 6

2 months ago
I have a simple fix for this, but it's not clear to me why it is required. I'm going spend a bit more time on this before putting it up for review.
Flags: needinfo?(dminor)
(Assignee)

Updated

2 months ago
Status: NEW → ASSIGNED
(Assignee)

Comment 7

2 months ago
Created attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

The fix for Bug 1409018 accidentally removed saving the current desktop
configuration during Init() which causes it to not be set when a different
screen is selected, meaning that regardless of the choice made, only the
first screen is captured.
(Assignee)

Comment 8

2 months ago
This fixes the problem I'm able to test directly (laptop + one external screen). I think it will also fix problems with two monitors, but I don't have the hardware to test that.
(Reporter)

Comment 9

2 months ago
I can help you with that testing, please let me know when I can start.
Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9006646 - Flags: review+
Blocks: 1409018

Comment 11

2 months ago
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0700ed6720e5
Save current desktop configuration in ScreenCapturerMac::Init; r=pehrsons

Comment 12

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0700ed6720e5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
I've added this as a known issue to the webrtc release notes for 62, https://wiki.mozilla.org/Media/WebRTC/ReleaseNotes/62
(Assignee)

Comment 14

2 months ago
(In reply to ovidiu boca[:Ovidiu] from comment #9)
> I can help you with that testing, please let me know when I can start.

This should be on the latest nightly, it would be great if you could test it with your setup. Thanks!
Flags: needinfo?(ovidiu.boca)
(Reporter)

Comment 15

2 months ago
Thanks Dan for reminding me about this.

I run an exploratory test around this issue on Mac OS X 10.12 with FF Nightly 64.0a1(2018-09-12) and I can't reproduce the initial problem, now the capture of the screens is working. I also found an issue, the quality of the capture from screen 2, not the default iMac monitor, it has some problems, please see the video: https://streamable.com/g6ytn
Please tell me your opinion and also how should we treat this issue, new bug or reopen this one?
Flags: needinfo?(ovidiu.boca) → needinfo?(dminor)
(Assignee)

Comment 16

2 months ago
(In reply to ovidiu boca[:Ovidiu] from comment #15)
> Thanks Dan for reminding me about this.
> 
> I run an exploratory test around this issue on Mac OS X 10.12 with FF
> Nightly 64.0a1(2018-09-12) and I can't reproduce the initial problem, now
> the capture of the screens is working. I also found an issue, the quality of
> the capture from screen 2, not the default iMac monitor, it has some
> problems, please see the video: https://streamable.com/g6ytn
> Please tell me your opinion and also how should we treat this issue, new bug
> or reopen this one?

Thank you for testing this. I think we should open a new bug for the screen capture quality issue, my first guess is that it is not related to the problem here, so I think it makes sense to investigate it separately.

It might be related to Bug 1409018, do you have time to do a quick regression window around when that landed? I did not see that problem when I tested the fix for this bug, so I'm not sure if I will be able to reproduce it, unless this is a recent regression that would not have shown up last week.
Flags: needinfo?(dminor)
(Assignee)

Comment 17

2 months ago
Of course you can't do a regression window, this bug would stop you from being able to see if the problem started at that time.
(Reporter)

Comment 18

2 months ago
I'll put an NI? for myself to do a regression.
Flags: needinfo?(ovidiu.boca)
Dan, should we consider it safe for a 63 beta uplift or can we let it ride the trains with 64? Thanks
Flags: needinfo?(dminor)
(Assignee)

Comment 20

2 months ago
Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1409018
[User impact if declined]:
User is unable to select second monitor for screen capture in OS X
[Is this code covered by automated tests?]:
No, we don't have multiple monitors in automation :/
[Has the fix been verified in Nightly?]:
Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
No
[List of other uplifts needed for the feature/fix]:
None
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
This is a one line fix that restores previous functionality.
[String changes made/needed]:
None.
Flags: needinfo?(dminor)
Attachment #9006646 - Flags: approval-mozilla-beta?
(Assignee)

Comment 21

2 months ago
(In reply to Pascal Chevrel:pascalc from comment #19)
> Dan, should we consider it safe for a 63 beta uplift or can we let it ride
> the trains with 64? Thanks

Yes, we want this on beta if at all possible. Thanks for reminding me!
Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Low-risk patch fixing a regression, uplift approved for 63 beta 6. Thanks
Attachment #9006646 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox-esr60: --- → unaffected

Comment 23

2 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/feca63da8d4f
status-firefox63: affected → fixed
Flags: qe-verify+
(Reporter)

Comment 24

2 months ago
I verified this issue on Mac OS X 10.12 with the latest FF Nightly 64.0a1(2018-09-13) and FF beta 63.0b6 and I can't reproduce the issue from the description based on that I will mark this as verified fixed. 
Please note that the issue described in comment 15 is still reproducilbe. I will try to find a regression range and file a new bug for it.
Status: RESOLVED → VERIFIED
status-firefox63: fixed → verified
status-firefox64: fixed → verified
Flags: qe-verify+
Flags: needinfo?(ovidiu.boca)
Do you think this is safe/important enough as a ride-along in 62.0.2?
Flags: needinfo?(dminor)
(Assignee)

Comment 26

2 months ago
(In reply to Julien Cristau [:jcristau] from comment #25)
> Do you think this is safe/important enough as a ride-along in 62.0.2?

Yes, that would be great!
Flags: needinfo?(dminor)
Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

OK let's take this for 62.0.2 based on comments 20 and 26.
Attachment #9006646 - Flags: approval-mozilla-release+

Comment 28

2 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/5e2fe8c6f242
status-firefox62: affected → fixed
(Reporter)

Comment 29

2 months ago
I tested this on Mac OS X 10.12 with FF 62.0.2 release candidate and I can confirm the fix.
status-firefox62: fixed → verified
You need to log in before you can comment on or make changes to this bug.