Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed logic for handling multiple controllable tabs #259

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Chetter2
Copy link

I found this plugin cool but a little bit uncomfortable to use with multiple controllable tabs. So I changed the logic so that it controls tabs in next order:
if the music is already playing in tabs then control command goes only in these tabs
else if active tab is controllable then command goes in the active tab
else command goes in last controlled tab

I found this much more comfortable for me. But I understand that this is a major change so feel free to reject.

@feedbee
Copy link
Collaborator

feedbee commented Mar 4, 2018

@Chetter2, thank you for great contribution. While such behaviour seems to be very logical, there is one significant disadvantage as well.

Let's assume that I listen to music in background tab. Then I open new Youtube tab and start movie playback there. Few seconds later I understand that I'm listening to both background music and Youtube sound in the same time. I want to set music on pause, but by pressing pause key I pause Youtube movie, and the music is still playing.

This problem is solvable. We can control sound playback from all tabs and stop playback in all other controllable tabs when playback starts in any controlled tab.

What do you think about it?

@Chetter2
Copy link
Author

Chetter2 commented Mar 4, 2018

There is no such disadvantage. Proposed logic will pause both movie and music because controlling already playing tabs have a higher priority. And additional pressing play button will resume the only movie because there are no playing tabs at the moment.

Glad to see this repository alive. I found my code inconsistent. And will update pull request soon.

@feedbee
Copy link
Collaborator

feedbee commented Mar 4, 2018

Ok, I'll test your version. It'll take some time.

@Chetter2
Copy link
Author

Chetter2 commented Mar 5, 2018

I've read a referenced thread and the blog post http://smus.com/remote-controls-web-media/ . Funny thing what we have the same idea, but @borismus go a little bit further. He used media focus stack instead of last playing tab, which is crucial for the case when last playing tab closing. So stack must be implemented before a merge.

For the case when

If I listen music with (for example) yandex.music.ru in the background tab and surfing social network with media support vk.com (for example), topmost tab will be vk.com, while I want to control yandex.music.ru. While your idea with media stack is very interesting, I think user need to have possibility to choose tab he wants to control manually. And it's a good practice to show user which tab is currently under control. Page action is the way to resolve it both.

The media control over vk can be easily disabled by clicking the page action button. And a user which want full control is the programmer's myth.

@feedbee
Copy link
Collaborator

feedbee commented Mar 5, 2018

I've been thinking about it a bit lately. And made a conclusion, that users should have possibility to choose what mode (strategy) they prefer. Keysocket extension has 13,990 weekly users. We should be very careful changing default behaviour as it may come out harmful for many people.

The best way to go further (but not the only) is to add options page and place there a selector of behaviour. And keep current behaviour as default for the fist time.

Possible behaviours:

  1. Current, all tabs are controllable, but user can enable/disable any tab manually.
  2. New, but still close to current. All tabs are controllable, but all are disabled by default. User can enable/disable any tab manually.
  3. Your version.

By the way, I've tested your code. It has worked good, but my controversial case has seemed not convenient for me. When something is playing at the background, and I open new tab and start playback there, I want to stop background playback pressing stop, not them both. It's a little bit different from you version.

But any way, I want to move further. We must implement and release new behaviour as close to the initial media focus idea as possible.

@SCaptainCAP
Copy link

I think there can be a setting for this behaviour. For me it will be more comfortable if the plugin was controlling only one tab of every opened domain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants