Sharing controls no longer shown on detached Hello window

VERIFIED FIXED in Firefox 42

Status

Hello (Loop)
Client
P1
normal
Rank:
14
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: adalucinet, Assigned: mikedeboer)

Tracking

({regression})

unspecified
mozilla44
regression
Points:
3

Firefox Tracking Flags

(firefox40 unaffected, firefox41+ wontfix, firefox42+ verified, firefox43+ verified, firefox44 verified)

Details

(Whiteboard: [sharing defect])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Reproducible with 41.0b1, latest Developer Edition 42.0a2 and Nightly 43.0a1 (e10s on/off)
Affected platforms: Windows 7 x64, Windows 10 x86, Mac OS X 10.10.4 and Ubuntu 14.04 x86

Steps to reproduce:
1. Start Firefox and Click on Hello icon (chat bubble with smiley face).
2. Click on "Start a conversation" and access "Share your screen" button.
3. Select "Share other windows".
4. Select a window to share and click "Share selected items".
5. Detach the conversation window.
6. Re-attach it.

Expected result: Shared screen icon is properly displayed.
Actual result: Question mark icon is displayed instead of Share screen icon. 

Addition notes:
1. Not reproducible with 40RC build 4.
2. When clicking the Question mark icon the correct icon is automatically displayed.
3. With Nightly 2015-07-19, Shared screen icon completely vanishes after re-attaching the conversation window. Will investigate further for a regression range.
(Reporter)

Comment 1

3 years ago
Regression range:

(m-c):
Last good revision: 7d4ab4a9febd
First bad revision: 4700d1cdf489
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7d4ab4a9febd&tochange=4700d1cdf489

(m-i):
Last good revision: ab9471275a90
First bad revision: 9ef529a9a02b
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ab9471275a90&tochange=9ef529a9a02b
I just confirmed via local testing, this is a regression from bug 1169629.

I've also realised there's a slightly worse case now - the sharing icons aren't shown on the popped out window.

Its also slightly sad that we don't seem to have automated tests for these.

[Tracking Requested - why for this release]: Regression in existing functionality that shows the user with which windows/tabs their devices/screen are being shared with.
Blocks: 1169629
Rank: 15
status-firefox40: --- → unaffected
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
tracking-firefox41: --- → ?
tracking-firefox42: --- → ?
tracking-firefox43: --- → ?
Priority: -- → P1
Summary: Question mark displayed instead of Share screen icon after re-attaching conversation window → Sharing controls no longer shown on detached Hello window / Question mark icon when re-attached

Comment 3

2 years ago
Tracked for all nom'd versions though I hope we can fix it in this beta cycle.
tracking-firefox41: ? → +
tracking-firefox42: ? → +
tracking-firefox43: ? → +
As I look through tracked bugs which don't have owners yet....

Mark do you want to take on this bug, and maybe add another bug for missing tests? 

Or mossop, since your bug is the regressing one?
Flags: needinfo?(standard8)
Flags: needinfo?(dtownsend)
Keywords: regression
I'm out on Monday, and I don't feel I have enough knowledge of how the notifications work. I would suggest Mossop or maybe :florian if he's about.
Flags: needinfo?(standard8)
Is this the same as bug 1196706?
Flags: needinfo?(dtownsend)
Now that I look, it does seem likely.
It might have the same cause, but I don't see that its quite the same - this is about the icons missing from the toolbar, but that's about the popup being displayed in the wrong place.
(In reply to Mark Banner (:standard8) from comment #2)

> Its also slightly sad that we don't seem to have automated tests for these.

I think that's bug 1132404.
(In reply to Dave Townsend [:mossop] from comment #6)
> Is this the same as bug 1196706?

(In reply to Mark Banner (:standard8) from comment #8)
> It might have the same cause, but I don't see that its quite the same - this
> is about the icons missing from the toolbar, but that's about the popup
> being displayed in the wrong place.

This is also independent of e10s whereas bug 1196706 is slated as e10s.
Flags: needinfo?(lhenry)
Flags: needinfo?(dtownsend)
Mike, you originally implemented notifications with no iconbox, can you work on this or direct it to someone who can?
Flags: needinfo?(dtownsend) → needinfo?(mdeboer)
(Assignee)

Comment 12

2 years ago
Sure, I'll take it.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 43.2 - Sep 7
Points: --- → 3
Flags: needinfo?(mdeboer)
Flags: needinfo?(lhenry)

Updated

2 years ago
Rank: 15 → 19
Seems too late to fix this in 41. This does not seem to be a release blocker either.
status-firefox41: affected → wontfix

Updated

2 years ago
Rank: 19 → 14
(Assignee)

Updated

2 years ago
Iteration: 43.2 - Sep 7 → 44.1 - Oct 5
(Assignee)

Updated

2 years ago
Summary: Sharing controls no longer shown on detached Hello window / Question mark icon when re-attached → Sharing controls no longer shown on detached Hello window
(Assignee)

Comment 14

2 years ago
Created attachment 8667221 [details] [diff] [review]
Patch v1: make sure that the notification icons and doorhangers are shown

This does the trick and I'm pretty sure this doesn't regress bug 1169629 - this is why I'm asking you to review this patch, Dave.
Attachment #8667221 - Flags: review?(dtownsend)
Comment on attachment 8667221 [details] [diff] [review]
Patch v1: make sure that the notification icons and doorhangers are shown

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

Looks like this works
Attachment #8667221 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 16

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/36cdf3559c3bea98a414e2f66916b2a5f496d9f5
Bug 1194706: make sure that the notification icons and doorhangers are shown in undocked chat windows too. r=Mossop
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4945854&repo=fx-team
Flags: needinfo?(mdeboer)

Updated

2 years ago
Whiteboard: [sharing defect]
(Assignee)

Comment 19

2 years ago
Created attachment 8672515 [details] [diff] [review]
Patch v1.1: make sure that the notification icons and doorhangers are shown

Dave, could you check if this doesn't regress your work in bug 1169629? Thanks!

(This is a reshuffle of the previous patch that makes all tests pass.)
Attachment #8667221 - Attachment is obsolete: true
Flags: needinfo?(mdeboer)
Attachment #8672515 - Flags: review?(dtownsend)
Comment on attachment 8672515 [details] [diff] [review]
Patch v1.1: make sure that the notification icons and doorhangers are shown

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

Looks good.
Attachment #8672515 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 23

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/4d726b108b2d288a8e25dbda86912116a8da7dfb
Bug 1194706: make sure that the notification icons and doorhangers are shown in undocked chat windows too. r=Mossop
https://hg.mozilla.org/mozilla-central/rev/4d726b108b2d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Mike, is this something you would want to uplift since 42 and 43 are also affected?
Flags: needinfo?(mdeboer)
(Assignee)

Comment 26

2 years ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #25)
> Mike, is this something you would want to uplift since 42 and 43 are also
> affected?

Yes, please!
Flags: needinfo?(mdeboer)
(Assignee)

Comment 27

2 years ago
Comment on attachment 8672515 [details] [diff] [review]
Patch v1.1: make sure that the notification icons and doorhangers are shown

Approval Request Comment
[Feature/regressing bug #]: Bug 1169629.
[User impact if declined]: User will not see webrtc notification icons in the titlebar when the conversation is undocked.
[Describe test coverage new/current, TreeHerder]: landed on m-c, tests pass.
[Risks and why]: minor.
[String/UUID change made/needed]: n/a.
Attachment #8672515 - Flags: approval-mozilla-beta?
Attachment #8672515 - Flags: approval-mozilla-aurora?
Comment on attachment 8672515 [details] [diff] [review]
Patch v1.1: make sure that the notification icons and doorhangers are shown

Fix for regression in Hello, no tests but ok on m-c. 
Approved for uplift to aurora and beta.
Attachment #8672515 - Flags: approval-mozilla-beta?
Attachment #8672515 - Flags: approval-mozilla-beta+
Attachment #8672515 - Flags: approval-mozilla-aurora?
Attachment #8672515 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 30

2 years ago
Confirming the fix on latest 44.0a1 (from 2015-10-14), across platforms [1]. While this is also fixed with latest 43.0a2 (from 2015-10-14) under Windows 7 64-bit and Ubuntu 14.04 32-bit, I can still *reproduce* under Mac OS X 10.10.4 and 10.11. Any ideas?
Also, another issue is that when 'No window' is selected to share, the call is automatically interrupted and ‘No camera or microphone found’ notification is displayed along with ‘Rejoin conversation’ button. Note that it isn't reproducible with latest 43.0a2. Do you have this under your radar? 

[1] Mac OS X 10.10.4, Ubuntu 14.04 32-bit and Windows 7 64-bit
Flags: needinfo?(mdeboer)
(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #30)
> Also, another issue is that when 'No window' is selected to share, the call
> is automatically interrupted and ‘No camera or microphone found’
> notification is displayed along with ‘Rejoin conversation’ button. Note that
> it isn't reproducible with latest 43.0a2. Do you have this under your radar? 

This is a separate bug that will be fixed by bug 1210754.
(Reporter)

Comment 33

2 years ago
Thanks, Mark!
Confirming the fix with 43.0a2 tinderbox build from 2015-10-15, under Mac OS X 10.10.4.
Status: RESOLVED → VERIFIED
status-firefox43: fixed → verified
status-firefox44: fixed → verified
Flags: needinfo?(mdeboer)
Flags: qe-verify+
(Reporter)

Comment 34

2 years ago
Confirming the fix with 42.0b7 (Build ID: 20151015151621) too, across platforms [1].

[1] Windows 7 64-bit, Mac OS X 10.10.4 and Ubuntu 14.04 32-bit
status-firefox42: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.