Fix play button case of default isPlaying.

[jaccarmac]

The V3 changes greatly simplified isPlaying, which changed the semantics slightly. This change flips the buggy first boolean case: When the play button is visible, music is not playing.

[jaccarmac]

@web-scrobbler/web-scrobbler-team: How important is it to restore the old semantics? Namely: The old method short-circuited only if the play button selector was defined and visible; This method short-circuits if the selector is defined and invisible, never checking the pause case in that case.

[yayuyokitano]

Whoops, this one slipped through! Good find.

[yayuyokitano]

@web-scrobbler/web-scrobbler-team: How important is it to restore the old semantics? Namely: The old method short-circuited only if the play button selector was defined and visible; This method short-circuits if the selector is defined and invisible, never checking the pause case in that case.

Had a look into the method pre/after v3, looks like it was a simplification that happened because of the misread of the original function, I doubt the difference is meaningful with this bug fixed, but to be safe I think it's a good idea to restore the old functionality fully.

[yayuyokitano]

@web-scrobbler/web-scrobbler-team: How important is it to restore the old semantics? Namely: The old method short-circuited only if the play button selector was defined and visible; This method short-circuits if the selector is defined and invisible, never checking the pause case in that case.

Had a look into the method pre/after v3, looks like it was a simplification that happened because of the misread of the original function, I doubt the difference is meaningful with this bug fixed, but to be safe I think it's a good idea to restore the old functionality fully.

I'm headed off to bed rn so won't be able to work on this until the morning, but if you want to go ahead and jump into it we already have an unexported visiblefilter function in content util that is based on the jquery is(":visible"). I think we can probably just export that, probably under a different name, and use it.

[inverse]

Great catch!

Do we have any connectors that handle both selectors?

[jaccarmac]

TIDAL does (though it seems to function with just the one); That's why I dug into the change beyond the obvious breakage.

[yayuyokitano]

As far as I can tell v2 also short-circuited, but it short circuited to false if selector defined but element not exists. v3 short circuits to true instead.

[inverse]

@yayuyokitano is correct - here is the v2 code.

If playbutton selector is set and it didnt match it''ll return false. Sounds like the TIDAL pauseButtonSelector should be removed to avoid confusion.

	this.isPlaying = () => {
		if (this.playButtonSelector) {
			const playButton = Util.queryElements(this.playButtonSelector);
			if (playButton) {
				return !playButton.is(':visible');
			}

			return false;
		}

		if (this.pauseButtonSelector) {
			const pauseButton = Util.queryElements(this.pauseButtonSelector);
			if (pauseButton) {
				return pauseButton.is(':visible');
			}

			return false;
		}

		/*
		 * Return true if play/pause button selector is not specified. It's
		 * better to assume the playback is always playing than otherwise. :)
		 */

		return true;
	};

FYI - a good place for exploring v2 is using https://github.dev/web-scrobbler/web-scrobbler/tree/588cb49ca49934a455664254f61741bb791f366c