alex_mayorga Reporter
	Description • 10 years ago

Pre-requisites:
- Plug an external monitor
- Move the Nightly window to secondary monitor

Steps:
1 Load https://appear.in/nightly
2 Click "Share Selected Devices" button

Result:
An orange indicator appears on the primary monitor and does nothing when clicked.

Expected result:
The orange indicator is clickable and lets you control sharing.

	Florian Quèze [:florian] Assignee
	Comment 1 • 10 years ago

(In reply to alex_mayorga from comment #0)

> An orange indicator appears on the primary monitor and does nothing when
> clicked.

Is there an error message in the browser console? ("Tools -> Web Developer -> Browser Console" on Mac; not sure where it is exactly on Windows.)

	:Gijs (he/him)
	Comment 2 • 10 years ago

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> (In reply to alex_mayorga from comment #0)
> 
> > An orange indicator appears on the primary monitor and does nothing when
> > clicked.
> 
> Is there an error message in the browser console? ("Tools -> Web Developer
> -> Browser Console" on Mac; not sure where it is exactly on Windows.)

	alex_mayorga Reporter
	Comment 3 • 10 years ago

The relevant bits from "Browser Console":

Unknown property '-moz-font-smoothing'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'font-smoothing'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'speak'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'background-position-y'.  Declaration dropped. f35ccfdc.main.css:1
Error in parsing value for 'display'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'webkit-flex-wrap'.  Declaration dropped. f35ccfdc.main.css:1
Unknown pseudo-class or pseudo-element '-webkit-full-screen'.  Ruleset ignored due to bad selector. f35ccfdc.main.css:1
Unknown pseudo-class or pseudo-element 'full-screen'.  Ruleset ignored due to bad selector. f35ccfdc.main.css:1
Error in parsing value for 'top'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'touch-callout'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property 'user-select'.  Declaration dropped. f35ccfdc.main.css:1
Error in parsing value for 'background'.  Declaration dropped. f35ccfdc.main.css:1
Unknown pseudo-class or pseudo-element '-webkit-input-placeholder'.  Ruleset ignored due to bad selector. f35ccfdc.main.css:1
Unknown pseudo-class or pseudo-element '-ms-input-placeholder'.  Ruleset ignored due to bad selector. f35ccfdc.main.css:1
Unknown pseudo-class or pseudo-element 'input-placeholder'.  Ruleset ignored due to bad selector. f35ccfdc.main.css:1
Error in parsing value for 'max-height'.  Declaration dropped. f35ccfdc.main.css:1
Error in parsing value for '-moz-transition'.  Declaration dropped. f35ccfdc.main.css:1
Error in parsing value for 'transition'.  Declaration dropped. f35ccfdc.main.css:1
Unknown property '-moz-osx-font-smoothing'.  Declaration dropped. font-awesome.min.css:4
Expected 'none' or URL but found 'progid'.  Error in parsing value for 'filter'.  Declaration dropped. font-awesome.min.css:4
"This appears to be Firefox" dedb9407.main.js:5
"ScreenShareAction: check-extension message: " undefined angular.min.js:86
"ChromeNotifierAction: check-extension message: " undefined angular.min.js:86
Error in parsing value for 'background-color'.  Declaration dropped. nightly
Expected end of value but found '{'.  Error in parsing value for 'border'.  Declaration dropped. nightly
Invalid URI. Load of media resource  failed. nightly
"ChromeNotifierAction: visited-room-data message: " Object { /pruebamia: Object, /alex_mayorga: Object, /nightly: Object } angular.min.js:86
"Updating stream: {stream.id: 0 stream.streamId: undefined}" angular.min.js:86
"Attaching media stream" dedb9407.main.js:5
Unknown property '-moz-border-radius'.  Declaration dropped. main.css:12
Unknown property '-moz-box-shadow'.  Declaration dropped. main.css:30
Unknown property '-moz-box-shadow'.  Declaration dropped. main.css:35
Unknown property '-moz-osx-font-smoothing'.  Declaration dropped. font-awesome.min.css:4
Expected 'none' or URL but found 'progid'.  Error in parsing value for 'filter'.  Declaration dropped. font-awesome.min.css:4
Expected media feature name but found 'touch-enabled'. appearin:1
Expected media feature name but found '-webkit-touch-enabled'. appearin:1
Expected media feature name but found '-o-touch-enabled'. appearin:1
Expected media feature name but found '-ms-touch-enabled'. appearin:1
Expected media feature name but found 'modernizr'. appearin:1
Unknown property '-moz-opacity'.  Declaration dropped. appearin
Error in parsing value for 'background-image'.  Declaration dropped. appearin
Expected color but found 'left'.  Error in parsing value for 'background-image'.  Declaration dropped. appearin

	:Gijs (he/him)
	Comment 4 • 10 years ago

Can you take a screenshot of the indicator, and can you clear the console, then click the indicator, and then check if anything appears there?

	alex_mayorga Reporter
	Comment 5 • 10 years ago

Expected media feature name but found '-webkit-min-device-pixel-ratio'. from DOM
Expected media feature name but found '-o-min-device-pixel-ratio'. from DOM
Expected media feature name but found 'min-device-pixel-ratio'. from DOM

	:Gijs (he/him)
	Comment 6 • 10 years ago

I'm confused. Is the indicator at the bottom of the screen? :-\

So the desired behaviour is that clicking the microphone should focus the Firefox window and drop down the doorhanger. Does it do neither of those things? (the errors you pasted are not related)

	alex_mayorga Reporter
	Comment 7 • 10 years ago

I'm referring to the white on orange button next to the Nightly logo.

Clicking it just make the Nightly window flash for a split second here.

	u279076
	Comment 8 • 10 years ago

I think what Alex is referring to here is the overlay icon that appears on the screen, outside of the Firefox window. When you load https://appear.in/nightly and share your devices, you will get a yellow "camera" icon in the awesomebar. This icon only ever appears in the window that instantiated it (this is normal AFAIK).

However, there is also a small overlay that appears at the top-center of the main screen. This overlay contains the application icon, a yellow camera icon, and a yellow microphone icon. Clicking this overlay triggers the sharing door hanger in the Firefox window that instantiated it. Even if the window is on the secondary monitor, this overlay will appear on the main screen.

	u279076
	Comment 9 • 10 years ago

I also checked on https://meet.tokbox.com/mozilla-qa and it does the same thing:

Device sharing overlay appears on the main screen and clicking it triggers the sharing doorhanger in the browser window on the screen where that window exists.

	:Gijs (he/him)
	Comment 10 • 10 years ago

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> I also checked on https://meet.tokbox.com/mozilla-qa and it does the same
> thing:
> 
> Device sharing overlay appears on the main screen and clicking it triggers
> the sharing doorhanger in the browser window on the screen where that window
> exists.

Right. This is the expected behaviour. Alex is saying clicking the indicator doesn't do anything. It seems you can't reproduce that...

	alex_mayorga Reporter
	Comment 11 • 10 years ago

On https://meet.tokbox.com/mozilla-qa the indicator does show the door hanger when clicked and this shows on "Browser Console":

A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Thu Aug 21 2014 16:15:05 GMT-0500 (Central Standard Time (Mexico))
Full Message: null

	:Gijs (he/him)
	Comment 12 • 10 years ago

(In reply to alex_mayorga from comment #11)
> On https://meet.tokbox.com/mozilla-qa the indicator does show the door
> hanger when clicked and this shows on "Browser Console":
> 
> A promise chain failed to handle a rejection. Did you forget to '.catch', or
> did you forget to 'return'?
> See
> https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/
> Promise
> 
> Date: Thu Aug 21 2014 16:15:05 GMT-0500 (Central Standard Time (Mexico))
> Full Message: null

What file name is associated with this error?

Anthony and Alex, do you perhaps have different equipment, ie one of you can/does only share a mic, and the other camera and microphone?

Alex, what if you test with: http://queze.net/goinfre/ff_gum_test.html ?

	alex_mayorga Reporter
	Comment 13 • 10 years ago

Bingo!

I think I've found the trigger for this bug.

I've set Flash to be click-to-play.

https://appear.in/ has a Flash elements that throw things off.
https://meet.tokbox.com/ has no Flash elements

	Florian Quèze [:florian] Assignee
	Comment 14 • 10 years ago

Ah, so is the doorhanger not opening when there's also a plugin icon displayed in the url bar?

Is this problem really specific to having the window on a secondary screen?

	u279076
	Comment 15 • 10 years ago

I can reproduce this now too, with Flash set to click-to-play on https://appear.in

This also seems to happen for me if the window is on the main screen so dual monitors seems to be a moot point.

	Florian Quèze [:florian] Assignee
	Comment 16 • 10 years ago

This fixes the bug reported here and another bug occuring with multiple notification icons shown at once.

Not requesting review yet because I haven't had time to verify that this doesn't break any test.

	Florian Quèze [:florian] Assignee
	Comment 17 • 10 years ago

(In reply to Florian Quèze [:florian] [:flo] from comment #16)
> and another bug occuring with multiple notification icons shown at once.

I moved this to its own bug: bug 1067367.

	Florian Quèze [:florian] Assignee
	Comment 18 • 10 years ago

Here is what's happening:
- the notification.reshow call from webrtcUI.jsm causes _update from PopupNotifications to be called with a specific notification and anchor specified.
Because of this, the doorhanger is briefly (not long enough to be visible, unless one is stepping through the code using a debugger) shown (the "showing" and "shown" events are notified to the notification's event callback).
- the browser window is activated (due to the browserWindow.focus() call in webrtcUI.jsm), the "activate" event is received by the PopopNotifications code which then calls _update without parameter out of a setTimeout. Without any specified anchor, the _update code will pick the first one (the click to play one in this case), see that all the notifications attached to that anchor are dismissed, decide that there's nothing to show and close the panel; which will cause the webrtc doorhanger notification to be dismissed (a "dismissed" event is received by the notification's event callback).

I think a reasonable solution here is to make _update called without parameter be a little smarter about which anchor it picks, ie. pick the first anchor that has non-dismissed notifications attached to it.

	Florian Quèze [:florian] Assignee
	Comment 19 • 10 years ago

Alternative steps to reproduce that don't involve click-to-play:
1. Load http://people.mozilla.org/~fqueze2/webrtc/
2. In about:config, add people.mozilla.org in the media.getusermedia.screensharing.allowed_domains list.
3. Click the "Screen & Audio" button.
4. Share a microphone and your screen.
5. See an orange microphone icon and an orange screensharing icon in the URL bar, and the global indicator popup window at the top of the screen.
6. Click the screen share icon of the global indicator.

Expected result:
The screensharing doorhanger should open.

Actual result:
The browser window gets focused, but the screensharing doorhanger isn't open.

Note: I only reproduced on Windows. Not sure if it's possible to reproduce on other OSes.

Note2: The reason why on https://appear.in/nightly the bug can only be reproduced if flash is blocked is that for some reason PopupNotifications._getNotificationsForBrowser returns the click-to-play notification before the webrtc one when flash is blocked, but after the webrtc notification when flash is allowed. I haven't further looked into what causes the ordering difference, but my guess is that it's related to in which order things are loaded.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 20 • 10 years ago

Comment on attachment 8489379 [details] [diff] [review]
Patch v2

Can you write a test for this?

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 22 • 10 years ago

Comment on attachment 8493780 [details] [diff] [review]
Patch v2 (with test)

Can you lower/raise the window rather than calling _update() directly in the test? I suppose that introduces greater change of intermittent failures :/

	Florian Quèze [:florian] Assignee
	Comment 23 • 10 years ago

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #22)

> Can you lower/raise the window rather than calling _update() directly in the
> test? I suppose that introduces greater change of intermittent failures :/

Yes, that would cause more asynchronicity and more chances of intermittent failures. Especially as the 'activate' event is handled from a setTimeout, and lowering the window would dismiss the notifications.

Try was green on the new test: https://tbpl.mozilla.org/?tree=Try&rev=5845cf9f75a3

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

	Florian Quèze [:florian] Assignee
	Comment 24 • 10 years ago

Comment on attachment 8493780 [details] [diff] [review]
Patch v2 (with test)

Approval Request Comment
[Feature/regressing bug #]: The bug has been here for a long time, but it became visible with bug 1037408 that landed for Firefox 33.
[User impact if declined]: Clicking the icons in the global indicator may not show the sharing doorhanger if there's another notification icon before (eg. the click to play icon).
[Describe test coverage new/current, TBPL]: the patch contains a test.
[Risks and why]: small risk as there's a small behavior change. If we were earlier in the cycle, I would request beta approval too, as the bug is visible on 33.
[String/UUID change made/needed]: none.

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/44a820e40ec5

	alex_mayorga Reporter
	Comment 26 • 10 years ago

Thanks for fixing this Florian =)

It now works on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140925030203 CSet: 1735ff2bb23e

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 27 • 10 years ago

Thanks for verifying this Alex. Setting correct status flag.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 28 • 10 years ago

Comment on attachment 8493780 [details] [diff] [review]
Patch v2 (with test)

Aurora+

	Florian Quèze [:florian] Assignee
	Comment 29 • 10 years ago

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

	Catalin Varga [QA][:VarCat]
	Comment 30 • 10 years ago

Verified as fixed using:

FF 34.10b
Build Id: 20141117202603
OS: Win 7 x64