Affected builds: - latest Aurora 44.0a2 - latest Nightly 45.0a1 Affected OS`s: - Windows 10 64-bit - Ubuntu 14.04 32-bit - Mac OS X 10.10 STR: 1. Start Firefox 2. Start a conversation 3. Press Pause to pause the sharing 4. Switch tabs Expected results: Don't quite know the intended behavior here but I think the Share bar should not change even if I switch through tabs, it should remain 'Paused' until one hits 'Resume' Actual results: Sharing is resumed on all tabs (does not matter which tab I focus). Notes: - This is not a regression, it was introduced when the Share bar was added, bug 1214214, or the bug where pause/unpause were added (could not find the bug).

Forgot to attach a GIF showing the issue.

Manuel, can you take this as a follow-up to bug 1214214?

Sure!

Hey Mike, I showed you the patch last week so the review will be quick. Thank you!

Comment on attachment 8697942 [details] [diff] [review] Tab sharing is unpaused when switching tabs Review of attachment 8697942 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. This was close to an r-, so please review your patch files yourself before posting. ::: browser/extensions/loop/bootstrap.js @@ +518,5 @@ > return; > } > > let box = gBrowser.getNotificationBox(); > + let pauseButtonLabel = this._browserSharePaused ? this._getString("infobar_button_resume_label") : I think it'd look best if you align the `this._getString(...` calls, each on their own line. Since you're alternating string IDs, perhaps it'd be better to put the conditional _inside_ `this._getString`: ```js let pauseButtonLabel = this._getString(this._browserSharePaused ? "infobar_button_resume_label" : "infobar_button_pause_label"); ``` @@ +538,4 @@ > isDefault: false, > callback: (event, buttonInfo, buttonNode) => { > + this._browserSharePaused = !this._browserSharePaused; > + bar.label = this._browserSharePaused ? this._getString("infobar_screenshare_paused_browser_message") : Same wrt alignment. @@ +558,5 @@ > } > }] > ); > > + // Sets 'paused' class if needed nit: missing '.'. @@ +561,5 @@ > > + // Sets 'paused' class if needed > + if (this._browserSharePaused) { > + bar.classList.toggle("paused", this._browserSharePaused); > + } Please remove the conditional and rewrite as: ```js bar.classList.toggle("paused", !!this._browserSharePaused); ``` ::: browser/extensions/loop/content/shared/js/activeRoomStore.js @@ +890,5 @@ > * > * @param {Number} windowId The new windowId to start sharing. > */ > _handleSwitchBrowserShare: function(windowId) { > + console.info(this); *ahum*

¡Hola Manuel! This just bite me on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20151217072309 CSet: 0711218a018d912036f7d3be2ae2649e213cfb85 Any hopes of a polished patch soon? ¡Gracias!

Comment on attachment 8700499 [details] [loop] mancas:bug1230946 > mozilla:master Keeping r+ from Mike with comments addressed.

Merged: https://github.com/mozilla/loop/commit/f108448d16d12bae41d49a760243e6e48a17714f https://github.com/mozilla/loop/commit/5a6ad7d9ea0f4b2fd32a89a00ec33dd3a4c56792 Mancas: You should have github access to land these yourself, but I've landed it for you given the afk.

Since this is marked fixed, I went ahead and tested on latest Nightly and the issue is still there. I see that no push has been made to Nightly and only merged in github (maybe this is because hello is now a add-on), is there a way I can test on this fix and others to come in latest Nightly?

Answered on irc: we're still getting processes set up. The add-on can currently be built from the repo.

I managed to reproduce this issue on Windows 10 x86, using Firefox 45.0a1(2015-10-07). The issue is no longer reproducible on Firefox 46.0a2(2016-03-02) and on Firefox 47.0a1(2016-03-02), using the default version of Hello(1.1.2 for Aurora build and 1.1.9 for Nightly build). The issue was verified on Windows 10 x86, Ubuntu 14.04 x64 and on Mac OS X 10.11.