Closed Bug 1107967 Opened 9 years ago Closed 9 years ago

"Stop sharing" doesn't work on teared off tabs

Categories

(Firefox :: General, defect)

34 Branch
defect
Not set
major
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 37
Iteration:
37.2
Tracking Status
firefox35 + verified
firefox36 + verified
firefox37 + verified

People

(Reporter: florian, Assigned: florian)

References

Details

(Keywords: regression)

Attachments

(1 file)

From the investigation in bug 1098437, it turns out I regressed this in 35 with bug 973001.

Impact of the bug:
- the Stop sharing UI doesn't work
- we are leaking the <browser> of the window were the tab lived before being teared off.
Flags: firefox-backlog+
[Tracking Requested - why for this release]: regression with possible privacy implications.
Attached patch FixSplinter Review
Turned out the "Stop sharing" action wasn't the only broken piece of the WebRTC UI after swapping tabs between windows. The "Allow" and "Deny" actions were also broken if a tab had been swapped with a pending gUM request.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attachment #8533914 - Flags: review?(felipc)
Points: --- → 3
Flags: qe-verify+
Iteration: --- → 37.2
Comment on attachment 8533914 [details] [diff] [review]
Fix

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

::: browser/modules/webrtcUI.jsm
@@ +73,5 @@
> +  swapBrowserForNotification: function(aOldBrowser, aNewBrowser) {
> +    this._streams.forEach(stream => {
> +      if (stream.browser == aOldBrowser)
> +        stream.browser = aNewBrowser;
> +    });

for (let .. of ..) instead of closure?
Attachment #8533914 - Flags: review?(felipc) → review+
https://hg.mozilla.org/mozilla-central/rev/6e31e40ecf17
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
Comment on attachment 8533914 [details] [diff] [review]
Fix

Approval Request Comment
[Feature/regressing bug #]: bug 973001
[User impact if declined]: Impossible to stop sharing a camera/microphone in a detached tab. Serious privacy issue.
[Describe test coverage new/current, TBPL]: Already on mozilla-central.
[Risks and why]: acceptable, compared to the regression.
[String/UUID change made/needed]: none.
Attachment #8533914 - Flags: approval-mozilla-beta?
Attachment #8533914 - Flags: approval-mozilla-aurora?
QA Contact: paul.silaghi
Attachment #8533914 - Flags: approval-mozilla-beta?
Attachment #8533914 - Flags: approval-mozilla-beta+
Attachment #8533914 - Flags: approval-mozilla-aurora?
Attachment #8533914 - Flags: approval-mozilla-aurora+
The patch here doesn't apply cleanly on mozilla-beta because bug 1095733 landed on 36 and isn't on 35.
(In reply to Florian Quèze [:florian] [:flo] from comment #7)
> The patch here doesn't apply cleanly on mozilla-beta because bug 1095733
> landed on 36 and isn't on 35.

Florian -- Does it make sense for me to ask for uplift on bug 1095733 to Fx35?
Flags: needinfo?(florian)
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #8)
> (In reply to Florian Quèze [:florian] [:flo] from comment #7)
> > The patch here doesn't apply cleanly on mozilla-beta because bug 1095733
> > landed on 36 and isn't on 35.
> 
> Florian -- Does it make sense for me to ask for uplift on bug 1095733 to
> Fx35?

I've unbitrotted my patch and landed it on beta anyway. For bug 1095733 that's up to you, but it doesn't seem to be a regression fix, so I wouldn't request uplift if it was my patch.
Flags: needinfo?(florian)
Verified fixed FF 36.0a2 (2014-12-15), 37.0a1 (2014-12-14) Win 7
Status: RESOLVED → VERIFIED
Verified fixed FF 35b4 Win 7
I think this bug fix may have created this one:  https://bugzilla.mozilla.org/show_bug.cgi?id=1122036

Any ideas on why isn't set notification.browser.messageManager on a Panel?
You need to log in before you can comment on or make changes to this bug.