1194706 - Sharing controls no longer shown on detached Hello window

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Ada [:adalucinet]]

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.

Comment 1

[Ada [:adalucinet]]

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

Comment 2

[Mark Banner (:standard8)]

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.

Comment 3

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked for all nom'd versions though I hope we can fix it in this beta cycle.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

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?

Comment 5

[Mark Banner (:standard8)]

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.

Comment 6

[Dave Townsend [:mossop]]

Is this the same as bug 1196706?

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Now that I look, it does seem likely.

Comment 8

[Mark Banner (:standard8)]

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.

Comment 9

[Florian Quèze [:florian]]

(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.

Comment 10

[Mark Banner (:standard8)]

(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.

Comment 11

[Dave Townsend [:mossop]]

Mike, you originally implemented notifications with no iconbox, can you work on this or direct it to someone who can?

Comment 12

[Mike de Boer [:mikedeboer]]

Sure, I'll take it.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Seems too late to fix this in 41. This does not seem to be a release blocker either.

Comment 14

[Mike de Boer [:mikedeboer]]

Attachment: 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.

Comment 15

[Dave Townsend [:mossop]]

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

Comment 16

[Mike de Boer [:mikedeboer]]

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

Comment 17

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/36f736832b9f

Comment 18

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4945854&repo=fx-team

Comment 19

[Mike de Boer [:mikedeboer]]

Attachment: 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.)

Comment 20

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c270e3ce380

Comment 21

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe11d644b0c7

Comment 22

[Dave Townsend [:mossop]]

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.

Comment 23

[Mike de Boer [:mikedeboer]]

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

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4d726b108b2d

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Mike, is this something you would want to uplift since 42 and 43 are also affected?

Comment 26

[Mike de Boer [:mikedeboer]]

(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!

Comment 27

[Mike de Boer [:mikedeboer]]

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.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/775552352d7a

Comment 30

[Ada [:adalucinet]]

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

Comment 31

[Mark Banner (:standard8)]

(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.

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/655607703bb1

Comment 33

[Ada [:adalucinet]]

Thanks, Mark!
Confirming the fix with 43.0a2 tinderbox build from 2015-10-15, under Mac OS X 10.10.4.

Comment 34

[Ada [:adalucinet]]

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