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

VERIFIED FIXED in Firefox 34

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Firefox 34
All
Windows 7
Points:
2
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox33 unaffected, firefox34+ verified, firefox35 verified)

Details

(Whiteboard: [screensharing-uplift])

Attachments

(1 attachment)

Assignee

Description

5 years ago
[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).
Assignee

Updated

5 years ago
Points: --- → 2
Assignee: nobody → florian
Status: NEW → ASSIGNED
Iteration: --- → 35.2

Updated

5 years ago
QA Contact: drno
Assignee

Comment 2

5 years ago
(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+
Assignee

Comment 3

5 years ago
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+
Assignee

Comment 6

5 years ago
For QA verification, see the steps to reproduce in bug 1063957.
Status: ASSIGNED → RESOLVED
Closed: 5 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
Assignee

Updated

5 years ago
Component: General → Device Permissions
You need to log in before you can comment on or make changes to this bug.