If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Window title in "windows or screen to share" list is not updated when navigating to another page

VERIFIED FIXED in Firefox 33

Status

()

Core
WebRTC: Audio/Video
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Philipp Hancke, Assigned: m_and_m)

Tracking

34 Branch
mozilla34
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox33 verified, firefox34 verified)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
steps to reproduce:
1) navigate to https://fippo.github.io/webrtc-landing/gum_test_fippo3.html
2) share a window (queries camera first)
3) navigate to https://talky.io, join a room
4) click "share screen"
5) note the "GUM Test Page - Nightly" in the "Window or screen to share:" list. This is from step 1 and not reflecting the current page title.

This looks like the list is generated during step 1 and cached.
The full list isn't cached (open a new window after 3 and before 4, and you'll see it in the list), but it appears it doesn't update the titles of existing windows when it Rescan()s.
Whiteboard: [screensharing-uplift]

Updated

3 years ago
Blocks: 1040061
Roman, Gian-Carlo, or Matt -- Would any of your feel comfortable taking this bug?  It's a follow on to the rescan bug (Bug 1041369) which we uplifted to Fx33.  I'd love to get this fixed within the next week (or sooner) and uplifted before we go to Beta, if possible.
Flags: needinfo?(rskalish)
Flags: needinfo?(linuxwolf)
Flags: needinfo?(gpascutto)
(Assignee)

Comment 3

3 years ago
I can take a look at it next week.
Flags: needinfo?(rskalish)
Flags: needinfo?(linuxwolf)
Flags: needinfo?(gpascutto)
Thanks, Matt!
Assignee: nobody → linuxwolf
(Assignee)

Comment 5

3 years ago
Created attachment 8480140 [details] [diff] [review]
Window title in "windows or screen to share" list is not updated when navigating to another page
(Assignee)

Updated

3 years ago
Attachment #8480140 - Flags: review?(rjesup)
Comment on attachment 8480140 [details] [diff] [review]
Window title in "windows or screen to share" list is not updated when navigating to another page

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

::: content/media/webrtc/MediaEngineWebRTCVideo.cpp
@@ +703,5 @@
> +void MediaEngineWebRTCVideoSource::Refresh(int aIndex) {
> +#ifdef MOZ_B2G_CAMERA
> +  mCaptureIndex = aIndex;
> +#else
> +  mCaptureIndex = aIndex;

a) hoist this outside the #ifdef
b) would this actually ever be different?  If it can't, then remove the parameter; if you're worried it might but shouldn't, MOZ_ASSERT or NS_WARN_IF

@@ +705,5 @@
> +  mCaptureIndex = aIndex;
> +#else
> +  mCaptureIndex = aIndex;
> +
> +  // assume the uniqueId hasn't changed ...

"Caller ensures this is the same UniqueId" or "Caller looked up this captureIndex by uniqueID, so it shouldn't change"
Attachment #8480140 - Flags: review?(rjesup) → review+
(Assignee)

Comment 7

3 years ago
(In reply to Randell Jesup [:jesup] from comment #6)
> Comment on attachment 8480140 [details] [diff] [review]
> Window title in "windows or screen to share" list is not updated when
> navigating to another page
> 
> Review of attachment 8480140 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/webrtc/MediaEngineWebRTCVideo.cpp
> @@ +703,5 @@
> > +void MediaEngineWebRTCVideoSource::Refresh(int aIndex) {
> > +#ifdef MOZ_B2G_CAMERA
> > +  mCaptureIndex = aIndex;
> > +#else
> > +  mCaptureIndex = aIndex;
> 
> a) hoist this outside the #ifdef
> b) would this actually ever be different?  If it can't, then remove the
> parameter; if you're worried it might but shouldn't, MOZ_ASSERT or NS_WARN_IF
> 

The index actually does change, at least for Mac when enumerating windows.
(Assignee)

Comment 8

3 years ago
Created attachment 8480175 [details] [diff] [review]
Window title in "windows or screen to share" list is not updated when navigating to another page
Attachment #8480175 - Flags: review?(rjesup)
(Assignee)

Updated

3 years ago
Attachment #8480140 - Attachment is obsolete: true

Updated

3 years ago
Attachment #8480175 - Flags: review?(rjesup) → review+
(Assignee)

Comment 9

3 years ago
Created attachment 8480228 [details] [diff] [review]
8480175: Window title in "windows or screen to share" list is not updated when navigating to another page

The previous patch had a stupid typo (#ifndef vs. #ifdef) that I thought hadn't been committed, but was.

My apologies for the noise /-:
Attachment #8480175 - Attachment is obsolete: true
Attachment #8480228 - Flags: review?(rjesup)

Updated

3 years ago
Attachment #8480228 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef2bf9308ed7
status-firefox33: --- → affected
status-firefox34: --- → affected
Target Milestone: --- → mozilla34
Backed out for various M3 failures (semi-random, UAF or leaving something running/leaked).
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ef2bf9308ed7

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fb2483f4928
(Assignee)

Comment 12

3 years ago
Created attachment 8481881 [details] [diff] [review]
v4 - Window title in "windows or screen to share" list is not updated when navigating to another page

Patch to address Mochitest-3 failures.  mCaptureIndex is changed when a device is allocated, meaning the enumeration index can be different.  However, the enumeration index is what is used to get the current device name.

Try of the interdiff: < https://tbpl.mozilla.org/?tree=Try&rev=3e0291fd3379 >
Failures in that run appear to be unrelated...
Attachment #8480228 - Attachment is obsolete: true
Attachment #8481881 - Flags: review?(rjesup)

Updated

3 years ago
Attachment #8481881 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c9024c845a9
Comment on attachment 8481881 [details] [diff] [review]
v4 - Window title in "windows or screen to share" list is not updated when navigating to another page

Approval Request Comment
[Feature/regressing bug #]: screensharing

[User impact if declined]: Wrong (old) window title shown

[Describe test coverage new/current, TBPL]: manual testing.  Green on inbound, works in manual testing.  Easily tested by hand

[Risks and why]: fairly simple patch.  Causes use confusion.

[String/UUID change made/needed]: none
Attachment #8481881 - Flags: approval-mozilla-aurora?

Updated

3 years ago
Keywords: qawanted
https://hg.mozilla.org/mozilla-central/rev/8c9024c845a9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
status-firefox34: affected → fixed
Attachment #8481881 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

3 years ago
Depends on: 1061475
DOn't uplift until bug 1061475 is fiixed and included, please.
https://hg.mozilla.org/releases/mozilla-aurora/rev/dd1c98e99569
status-firefox33: affected → fixed
Flags: qe-verify+
Reproduced the initial issue on old Nightly (2014-08-20), verified that the issue is fixed on Windows 7 64bit, Ubuntu 14.04 64bit and Mac OS X 10.9.4 using latest Aurora and Firefox 33 beta 2.
Status: RESOLVED → VERIFIED
status-firefox33: fixed → verified
status-firefox34: fixed → verified
Keywords: qawanted

Updated

3 years ago
Whiteboard: [screensharing-uplift]
You need to log in before you can comment on or make changes to this bug.