Closed Bug 1064909 Opened 10 years ago Closed 10 years ago

ensure the menubar webrtc indicator doesn't use strings that don't exist on Firefox 34

Categories

(Firefox :: Site Permissions, defect)

All
Windows 7
defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
35.3
Tracking Status
firefox33 --- unaffected
firefox34 + verified
firefox35 --- verified

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [screensharing-uplift])

Attachments

(1 file)

[Tracking Requested - why for this release]: WebRTC menubar sharing indicator broken on 34 in some edge cases.

This bug is on Windows and Linux only, on the 34 branch only (introduced by bug 1043372).

- We miss the strings for the case where the camera and either a screen or a window is shared, but NOT the microphone. See bug 1063957 for the trunk fix.

The suggested aurora solution for this case is:
(From :Gijs Kruitbosch in bug 1063957 comment #4)
> Can we lie and make camera + something else imply microphone is
> shared as well? I'd rather have the code be broken that way than not showing
> anything...

- It seems if a window and a screen are shared by the same tab, we are going to attempt to use a string that doesn't exist. This was fixed on trunk by http://hg.mozilla.org/mozilla-central/rev/ffc10f34de13#l5.143 from bug 1057006.
Flags: qe-verify+
Flags: firefox-backlog+
(In reply to Florian Quèze [:florian] [:flo] from comment #0)
> [Tracking Requested - why for this release]: WebRTC menubar sharing
> indicator broken on 34 in some edge cases.
> 
> This bug is on Windows and Linux only, on the 34 branch only (introduced by
> bug 1043372).
> 
> - We miss the strings for the case where the camera and either a screen or a
> window is shared, but NOT the microphone. See bug 1063957 for the trunk fix.
> 
> The suggested aurora solution for this case is:
> (From :Gijs Kruitbosch in bug 1063957 comment #4)
> > Can we lie and make camera + something else imply microphone is
> > shared as well? I'd rather have the code be broken that way than not showing
> > anything...
> 
> - It seems if a window and a screen are shared by the same tab, we are going
> to attempt to use a string that doesn't exist. This was fixed on trunk by
> http://hg.mozilla.org/mozilla-central/rev/ffc10f34de13#l5.143 from bug
> 1057006.

Yeah, if this is the case then I think we should fix that by just listing the screen; the window is implicitly included in that, I would expect (there's the edge-case of multiple screens and I don't know how we behave there, but it doesn't seem terribly important for now, considering a better fix is incoming).
Points: --- → 2
Assignee: nobody → florian
Status: NEW → ASSIGNED
Iteration: --- → 35.2
QA Contact: drno
Attached patch Aurora workaround (deleted) — Splinter Review
(In reply to Florian Quèze [:florian] [:flo] from comment #0)

> - It seems if a window and a screen are shared by the same tab, we are going
> to attempt to use a string that doesn't exist. This was fixed on trunk by
> http://hg.mozilla.org/mozilla-central/rev/ffc10f34de13#l5.143 from bug
> 1057006.

It seems I was confused when I wrote that paragraph: bug 1057006 landed on 34 too, so this should be fine. Only bug 1063957 needs to be worked around on 34/aurora.
Attachment #8494586 - Flags: review?(gijskruitbosch+bugs)
Attachment #8494586 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8494586 [details] [diff] [review]
Aurora workaround

Approval Request Comment
[Feature/regressing bug #]: bug 1043372
[User impact if declined]: In the case of a webpage using a camera and screenshare, but not the microphone, the keyboard accessible menu would be broken.
[Describe test coverage new/current, TBPL]: will be verified by QA.
[Risks and why]: low risk.
[String/UUID change made/needed]: The correct fix (bug 1063957) required 3 new strings. The patch here is a workaround for aurora to avoid needing these strings on 34.
Attachment #8494586 - Flags: approval-mozilla-aurora?
Iteration: 35.2 → 35.3
Comment on attachment 8494586 [details] [diff] [review]
Aurora workaround

Aurora+
Attachment #8494586 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
For QA verification, see the steps to reproduce in bug 1063957.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Reproduced the initial issue on Windows 64bit and Ubuntu 14.04 32bit using old Nightly (2014-09-06), verified that using Firefox 34beta9 and latest Developer Edition build the issue is fixed.
Status: RESOLVED → VERIFIED
Component: General → Device Permissions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: