Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Add jazzradio connector #20

Merged
merged 6 commits into from May 22, 2019
Merged

Add jazzradio connector #20

merged 6 commits into from May 22, 2019

Conversation

doronbehar
Copy link
Contributor

Hey @f1u77y ,

I was wondering whether you could help me please in setting up a connector for a website I wish to control with your extension - http://jazzradio.com/ .

Here's what I can say on my progress so far:

Using the 'Inspect element' feature of Firefox, I could rather easily find promising CSS selectors for each click()able element that could be used by the functions in the base-constructor. Notably, these are the ones:

this.playButtonSelector = '.icon-play';
this.stopButtonSelector = '.icon-pause';
this.artistSelector = '.artist-name';
this.titleSelector = '.track-name';
this.lengthSelector = '.total';
this.currentTimeSelector = '.time';
this.artSelector = '#art';

I've also inspected the base-connector code and I'm pretty sure they should work, even the complex ones like artSelector should be parsed in base-connector.js exactly as in every other website so I don't see a need to override these methods.

However, when I test this, the extension always says when I'm inside the website that it's 'stopped' - according to the tests and according to playerctl.

I've even tried to override some of them as so:
    get controlsInfo() {
        console.log('writing controlsInfo');
        return Promise.resolve({
            canGoNext: false,
            canGoPrevious: false,
            canPlay: true,
            canPause: true,
            canSeek: false,
            canControl: true,
        });
    }
    get playbackStatus() {
        console.log('trying playbackStatus');
        return Utils.query(this.playButtonSelector).then(elem => {
            console.log('found playButtonSelector: ' + elem);
            return 'paused';
            // return 'playing';
        });
    }
    play() {
        console.log('trying to click playButtonSelector');
        Utils.queryClick(this.playButtonSelector);
    }
    pause() {
        console.log('trying to click pauseButtonSelector');
        Utils.queryClick(this.pauseButtonSelector);
    }

But no luck. I couldn't even get those console messages to appear in the browser console when I ran the appropriate playerctl commands. I've had success with this sort of debugging mechanism back when I worked on the invidious connector.

Never the less, I've sort of proved my self the connector is even loaded thanks to the icon that appears at the URL bar in the website and when I added a console.log message at the constructor.

Anyway, I hope you'll be able to help me a little bit because I really enjoy this website and most importantly it's metadata could be very useful to me. I hope you like Jazz!

@f1u77y
Copy link
Owner

f1u77y commented May 22, 2019

I've noticed some more or less obvious things like the lack of get playbackStatus() definition or utilizing one from BaseConnector. But even after I've tried to fix them, connector doesn't seem to work. I'll try to look into this issue more carefully.

@doronbehar
Copy link
Contributor Author

doronbehar commented May 22, 2019

Thanks for taking your time to try this yourself and respond. Today I've looked even deeper into this and I tried to add debug messages in more places, in the base connector and other places but still no luck.

However, I was considering trying to use an injected script. I noticed deezer has one which is rather simple. I couldn't inspect those of yandex because I don't live in Russia :) Anyway, I was wondering, what brought up the need for such injection in the first place? Looking at deezer's injected script, I couldn't imagine it could do something I can't imagine the connector do..

BTW, for the sake of honesty, I'll note that I don't have any serious experience like yours in browser extensions development or JS in general. I only have 1 very simple real world extension I forked and I maintain by myself - https://github.com/doronbehar/invidious-redirect .

EDIT:

But as for this feature, I sure will try as hard as I can and I do understand concepts like promises and OOP..

@alexesprit
Copy link
Collaborator

alexesprit commented May 22, 2019

Works for me after applying these changes:

jazzradio.patch (Click to expand)
diff --git a/connectors/jazzradio.js b/connectors/jazzradio.js
index fafb1b3..215a5c1 100644
--- a/connectors/jazzradio.js
+++ b/connectors/jazzradio.js
@@ -16,5 +16,17 @@ new class extends BaseConnector {
         this.lengthSelector = '.total';
         this.currentTimeSelector = '.time';
         this.artSelector = '#art';
+
+        Utils.query('#row-player-controls').then(elem => this.observe(elem));
+    }
+
+    get playbackStatus() {
+        return Utils.query('#play-button a').then(icon => {
+            if (icon.classList.contains('icon-pause')) {
+                return 'playing';
+            } else {
+                return 'paused';
+            }
+        });
     }
 };
Screenshot (Click to expand)

image

The only issue is an artist title suffixed by dash symbol.

Also, there're other players with (almost) the same layout:

So, you can support all of these ones by using only one connector.

@doronbehar
Copy link
Contributor Author

Boy! That's certainly a good start 😄 . I can't describe to you in words how gratified I am. Thanks a lot!

Fortunately, most of functionality works by now after a few some small tweaks, thanks to the wonderful generic base-connector (excellent overall design BTW - very inviting for contributions!). I'm pushing the last changes.

There's only 1 thing that kind of bothers me right not but perhaps it's not that important: playerctl play doesn't work when the player is stopped. Usually, if autoplay is enabled, as it is by default in the browser's settings, the player starts playing automatically.

I was hoping that since the play() method from the base-connector runs playPause() if the status is not playing, this would work. But perhaps because some parts of the player is not fully loaded the status is not defined and this.playbackStatus.then is rejected? Not sure how to bypass this situation elegantly..

Thanks to @f1u77y, this new method override kicked of the new connector.
The artist selector needed to be trimmed from trailing ` - $` as well.
connectors/jazzradio.js Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor Author

Thank you so much @alexesprit for your help. I'd have never picked up myself that I need this line in the constructor:

Utils.query('#row-player-controls').then(elem => this.observe(elem));

In order to get things actually working. I also understand now why this is needed and why I can't control the player from outside if any of the elements inside #row-player-controls aren't changed from the start - when autoplay is blocked by the browser.

Therefor, I'm willing to suffer this small misbehavior since when the player starts playing, everything works.

I think this is ready for merge / review now @f1u77y , I'm taking the WIP out of the title.

Thanks again for both of you for your help :)

@doronbehar doronbehar changed the title [WIP] Add jazzradio connector Add jazzradio connector May 22, 2019
README.md Show resolved Hide resolved
@f1u77y
Copy link
Owner

f1u77y commented May 22, 2019

Sorry, I haven't noticed at the beginning, but di.fm doesn't seem to work, is not mentioned by alexesprit and has at least somewhat different design from other 3 websites (all of those seem to work). Should it really use this connector?

@doronbehar
Copy link
Contributor Author

Right.. Thanks for testing that, I thought it'd have a similar interface because it was available in the list at the bottom of jazzradio.com where all the other sites are linked as well. TBH, I don't really care about di.fm so I'll just drop it from the PR. Long live classical music and Jazz :]

@f1u77y f1u77y merged commit 60b527c into f1u77y:master May 22, 2019
@doronbehar
Copy link
Contributor Author

Awesome. Thanks!

@f1u77y
Copy link
Owner

f1u77y commented May 22, 2019

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants