Skip to content

Fix null issue in url youtube url when running a playlist with no vid… #31403

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

Merged
merged 5 commits into from
Jun 25, 2025

Conversation

mattiLeBlanc
Copy link
Contributor

…eoId

When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default.

By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v) which is causing an Invalid Video Id error for consumers.

…eoId

When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default.

By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (`ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v`) which is causing an Invalid Video Id error for consumers.
@mattiLeBlanc mattiLeBlanc requested a review from a team as a code owner June 24, 2025 02:18
@mattiLeBlanc mattiLeBlanc requested review from wagnermaciel and ok7sai and removed request for a team June 24, 2025 02:18
Copy link

google-cla bot commented Jun 24, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mattiLeBlanc
Copy link
Contributor Author

I have just signed the CLA

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

The change looks good to me, but there are a few issues:

  1. The CLA doesn't show up as signed.
  2. The linter is failing.
  3. Some unit tests are failing.

If it's easier, I can also land these changes separately.

@@ -587,17 +587,21 @@ export class YouTubePlayer implements AfterViewInit, OnChanges, OnDestroy {

// Important! We need to create the Player object outside of the `NgZone`, because it kicks
// off a 250ms setInterval which will continually trigger change detection if we don't.
const params: any = {
Copy link
Member

Choose a reason for hiding this comment

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

This can be typed as YT.PlayerOptions, rather than any.

apply YT.PlayerOptions to params
@mattiLeBlanc
Copy link
Contributor Author

@crisbeto let me know if it is alright now.

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

It's almost there. It looks like the formatting in the youtube-player.ts is off.

@mattiLeBlanc
Copy link
Contributor Author

It's almost there. It looks like the formatting in the youtube-player.ts is off.

Yeah, I am doing it inline so I can't format it. I haven't downloaded the full repo to my mac, I was a bit lazy. For the linter I ma have to do that.

@mattiLeBlanc
Copy link
Contributor Author

@crisbeto Formatting should now be resolved. My comments went out of the boundary and some other comments also just crossed over so I corrected that.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed labels Jun 25, 2025
@crisbeto crisbeto merged commit 2c87ec3 into angular:main Jun 25, 2025
24 of 25 checks passed
@crisbeto
Copy link
Member

The changes were merged into the following branches: main, 20.0.x

crisbeto pushed a commit that referenced this pull request Jun 25, 2025
…ideoId (#31403)

* Fix null issue in url youtube url when running a playlist with no videoId

When playing a playlist via playerVars.list = [playlistId] we dont have a videoId available yet. Plus we don't need one, Youtube will start at the first video of the playlist by default.

By injecting an undefined videoId in the YT Player constructur, we end up with null value in the iframe url (`ps://www.youtube.com/embed/null?cc_load_policy=1&enablejsapi=1&controls=1&showinfo=0&rel=0&listType=playlist&list=PLxf07yK_HAvdwhW-L0X3uP5aMe6eEt50v`) which is causing an Invalid Video Id error for consumers.

* Update youtube-player.ts

apply YT.PlayerOptions to params

* Update youtube-player.ts

fix formatting

* Update youtube-player.spec.ts

fix unit test

* Update youtube-player.ts

Fix formatting

(cherry picked from commit 2c87ec3)
@mattiLeBlanc mattiLeBlanc deleted the patch-1 branch June 25, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants