1137603 - WebRTC sharing notifications fail to open from the global indicator when the Hello window has been detached

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect)

Description

[Bogdan Maris, Desktop QA]

Affected builds:
- Latest Nightly
- Latest Aurora

Affected OS`s:
- Windows 7 64-bit
- Windows 8 64-bit
- Mac OS X 10.9.5
- Ubuntu 14.04 32-bit

STR:
1. Start a conversation
2. Detach Hello conversation window
3. From global indicator select one item (webcam)

4. Attach the conversation window
5. From global indicator select one item or more

6. Detach Hello conversation window

Actual results:
At step 3 the pop-up is misplaced
At step 5 nothing happens
After the second detachment the pop-ups appear.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

I believe "global indicator" referred tp is the top-of-screen "you're sharing mic and/or webcam" indicator.  Note this is Loop/Hello, so the doorhanger popped up isn't the normal urlbar doorhanger

Comment 3

[Florian Quèze [:florian]]

This looks related to bug 1107967 (fixed several months before this bug was reported though :-/).

Comment 4

[Florian Quèze [:florian]]

Attachment: Patch

The notifications didn't open correctly because the 'owner' property wasn't updated correctly when swapping the browsers. This happened because _swapBrowserNotifications from PopupNotifications.jsm was called with ourBrowser and otherBrowser reversed.

Comment 5

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8589690 [details] [diff] [review]
Patch

run through try?

Comment 6

[Florian Quèze [:florian]]

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

Comment 7

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/d9fca27fc251

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d9fca27fc251

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8589690 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: WebRTC sharing UI for Hello.
[User impact if declined]: WebRTC sharing indicator impossible to open from the global sharing indicator for the Hello chat window if the chat window has been detached and reattached.
[Describe test coverage new/current, TreeHerder]: QA will verify.
[Risks and why]: Low, the patch is a one-liner.
[String/UUID change made/needed]: none.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8589690 [details] [diff] [review]
Patch

Should be 38 beta 3 (or 4)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c9034098f9e1

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d4ee3499fe0d

Comment 13

[Bogdan Maris, Desktop QA]

Attachment: Screenshot showing the tiny pop-up

Verified that the initial issue is fixed on Ubuntu only.
I can`t verify this fix on Windows because of bug 1137602, and I found a new behavior on Mac (tiny and unusable pop-up):

1. From conversation click for eg. webcam icon
2. Detach conversation
3. Click icon 
(This happens if icons from Mac menubar are used as well)

Note:
I can only reproduce this on Mac and I seem to only reproduce on builds after this fix.

Should I log a new bug or reopen this one?

Comment 14

[Florian Quèze [:florian]]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #13)
> Created attachment 8600875 [details]
> Screenshot showing the tiny pop-up

Did you move the detached window around between a retina and a non-retina screen?

> Should I log a new bug or reopen this one?

New bug please.

Comment 15

[Bogdan Maris, Desktop QA]

(In reply to Florian Quèze [:florian] [:flo] from comment #14)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #13)
> > Created attachment 8600875 [details]
> > Screenshot showing the tiny pop-up
> 
> Did you move the detached window around between a retina and a non-retina
> screen?

Nope, I did not move the detached window at all and I don`t have a retina display (using a iMac 21.5 inch Late 2013)
 
> > Should I log a new bug or reopen this one?
> 
> New bug please.

Logged bug 1161023.

Closing this as verified fixed then.

Comment 16

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Verification on Beta should be enough - removing qe-verify flag.