Skip to content

Conversation

@nicholasbrantley
Copy link

@nicholasbrantley nicholasbrantley commented Oct 26, 2025

This is for #144, #145, and #146. It adds these embeds:

  • Apple Music: Album, Artist, Playlist, Track
  • Qobuz: Album, Track
  • Amazon Music

Additional fixes:

  • Fixed a bug where audio element heights were being changed and displaying incorrectly due to the aspect ratio calculations (aspect ratio is only calculated for video content now)
  • Corrected heights of Tidal track and Spotify track embeds
  • Correctly rounded corners of Deezer and Spotify embeds

Some notes:

  • I based them off the existing Spotify service
  • I didn't add anything to the fetchFactory file, but these services might have oembed support?
  • I ask if someone could check if I made the test files correctly.

@nicholasbrantley nicholasbrantley marked this pull request as ready for review October 28, 2025 02:27
Copy link
Member

@octfx octfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this PR!

Overall looks very good, two things:

  1. Do you have information on the zone parameter used by Qobuz? Can US-en be used as a "default" value worldwide?
  2. Regarding the added corner radii, I am currently undecided on if they should be kept. Omitting lets users style the embed, keeping them honors the brand style of the embed

I am currently leaning towards omitting the added border radius, but I am open for discussion.
If we keep it, then we should at least change from px to rem 🙂

@nicholasbrantley
Copy link
Author

Thank you very much for this PR!

Overall looks very good, two things:

1. Do you have information on the `zone` parameter used by Qobuz? Can `US-en` be used as a "default" value worldwide?

2. Regarding the added corner radii, I am currently undecided on if they should be kept. Omitting lets users style the embed, keeping them honors the brand style of the embed

I am currently leaning towards omitting the added border radius, but I am open for discussion. If we keep it, then we should at least change from px to rem 🙂

I will try to see if I can make a Qobuz account for an different region to see how the zone and track IDs change and how that affects it.

For the radii, I believe it would be better to default to having the rounded corners due to an iframe/prefers-color-scheme mistmatch quirk in all modern browsers. This occurs for MediaWiki skins that use the CSS prefers-color-scheme for setting light/dark theming (this is used for matching the website to the OS or browser color theme settings).

These service providers did not set their color scheme meta attribute in the header: <meta name="color-scheme" content="light dark" />. Without it, if there is a mismatch between the host site's preferred color scheme and the iframe's (by having no color-scheme meta, the browser assumes the iframe only supports light mode), the browser will set an unremovable background color on the iframe. I think it's supposed to be for readability in iframes that only have text, since the browser isn't supposed to modify iframe styling.

In the case of the music embeds, when the theme is dark, the browser adds a white background, which makes the corners appear like this:

image image image

The service providers just set a border radius on the default embed code you get from them. I think it's to cover compatibility with older browsers that do not support the color-scheme CSS property? Though they really should have added the meta tag anyway. There also isn't a way to remove the forced background color with CSS.

@nicholasbrantley
Copy link
Author

nicholasbrantley commented Oct 31, 2025

I am currently leaning towards omitting the added border radius, but I am open for discussion. If we keep it, then we should at least change from px to rem 🙂

I had just grabbed from their default embed code which used px. Looking deeper, I see that Apple, Amazon, and Deezer use px in the inner HTML radius, but Spotify mismatched and used rem on the inside.

1rem doesn't always equal 16px if the browser's default font size is changed, so a rem border radius would change with the font size. I think we should definitely update Spotify's to rem so it matches the inner styling, but maybe keep px for the others so that if the font size is being increased, the user doesn't see the embed slowly turn into a perfect circle which could eventually start cutting off content.

It's definitely an edge case, and I am not aware of how often people change their browser's default font size. I also wonder if on iOS and Android, do browsers use the OS font size as their default? I think more people change that one.

@alistair3149
Copy link
Collaborator

@nicholasbrantley Just to accelerate the review a bit, would you be able to share the wikitext you used for the test page?
That will help @octfx or me to spin up a test page quickly for the review.

@nicholasbrantley
Copy link
Author

nicholasbrantley commented Nov 22, 2025

@nicholasbrantley Just to accelerate the review a bit, would you be able to share the wikitext you used for the test page? That will help @octfx or me to spin up a test page quickly for the review.

Sure thing, below is examples for the affected services. You can also view them implemented on this wiki page.

== Apple Music ==

=== Album ===

{{#ev:applemusicalbum|1571062431}}

=== Track ===

{{#ev:applemusictrack|1571062737}}

=== Artist ===

{{#ev:applemusicartist|925515043}}

=== Playlist ===

{{#ev:applemusicplaylist|pl.f4d106fed2bd41149aaacabb233eb5eb}}

== Amazon Music ==

=== Album ===

{{#ev:amazonmusic|B09726942J}}

=== Track ===

{{#ev:amazonmusic|B09726DDC8}}

=== Artist ===

{{#ev:amazonmusic|B00O5YY63O}}

=== Playlist ===

{{#ev:amazonmusic|B0FB2KR28H}}

== Qobuz ==

=== Album ===

{{#ev:qobuzalbum|kq2pg1d0adhgb}}

=== Track ===

{{#ev:qobuztrack|123405941}}

== Corner radius updates ==

=== Spotify ===

{{#ev:spotifyalbum|1jUGmnsp1VAmqeYWgpHtK1}}

=== Deezer ===

{{#ev:deezeralbum|236706292}}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants