Enable the use of slug in the embed by tomekstach · Pull Request #144 · h5p/h5p-wordpress-plugin

tomekstach · 2022-07-07T08:26:03Z

Insert method option might be used in the embed to get an item by slug or ID.

bernhardkaindl · 2023-08-06T10:38:12Z

@icc, can you review this or somebody else?

otacke

Hi! I was made a community maintainer for H5P Group's repositories a few weeks ago, and I am still in the process trying to tackle all the open issues and pull requests.

I am terribly sorry that you have not received a reply before.

So, a late thank you in the name of H5P Group (I am not associated with them though). I'd have some small change requests, see comments. But your code absolutely does what it is supposed to, and it definitely makes sense to add it IMHO.

otacke · 2026-07-28T10:31:28Z

+
+    if (!empty($slug)) {
+      $q=$wpdb->prepare(
+        "SELECT  id ".


Could you please use the formatting of other queries for consistency, see .e.g. https://github.com/katalysteducation/h5p-wordpress-plugin/blob/d9fa94a9b48f7124dab2fd34e7536a563d781730/admin/class-h5p-plugin-admin.php#L791-L798

otacke · 2026-07-28T11:18:14Z

      'fullScreen' => $content['library']['fullscreen'],
      'exportUrl' => get_option('h5p_export', TRUE) ? $this->get_h5p_url() . '/exports/' . ($content['slug'] ? $content['slug'] . '-' : '') . $content['id'] . '.h5p' : '',
-      'embedCode' => '<iframe src="' . admin_url('admin-ajax.php?action=h5p_embed&id=' . $content['id']) . '" width=":w" height=":h" frameborder="0" allowfullscreen="allowfullscreen" title="' . esc_attr($title) . '"></iframe>',
+      'embedCode' => '<iframe src="' . admin_url('admin-ajax.php?action=h5p_embed&' . $identifier) . '" width=":w" height=":h" frameborder="0" allowfullscreen="allowfullscreen" title="' . $title . '"></iframe>',


The $title should still be passed through esc_attr to prevent broken HTML from being exploited (to mitigate XSS attacks).

$identifier should be passed through esc_attr as well. I know that the slug will have been processed before anyway, but anything that's user-provided should be sanitized. It's part of automated checks that Automattic runs against plugin code and they will complain about it.

otacke · 2026-07-28T11:44:00Z


    // Find content
-    $id = filter_input(INPUT_GET, 'id', FILTER_SANITIZE_NUMBER_INT);
+    $slug = filter_input(INPUT_GET, 'slug', FILTER_SANITIZE_ADD_SLASHES);


Is FILTER_SANITIZE_ADD_SLASHES the right choice here? The $slug is run through $wpdb->prepare later and will be escaped again, won't it? I think that could cause trouble with retrieval. FILTER_SANITIZE_STRING should suffice, right?

Hold the press ... I just checked https://www.php.net/manual/en/filter.constants.php#constant.filter-sanitize-string to be sure and noticed that it has been deprecated. Seems like

$slug = filter_input(INPUT_GET, 'slug', FILTER_DEFAULT, FILTER_FLAG_NO_ENCODE_QUOTES);

would be the way to go here?!

Or maybe in that way:

$slug = filter_input(INPUT_GET, 'slug', FILTER_VALIDATE_REGEXP, [ 'options' => ['regexp' => '/^[a-z0-9-]+$/i'] ]);

That should even work better, yes! I am just nor entirely sure: Can WordPress slugs use underscores, too? If yes, then that would be missing, right?

Yes, you are absolutely right. Done.

otacke · 2026-07-28T11:44:40Z

+      'embedCode' => '<iframe src="' . admin_url('admin-ajax.php?action=h5p_embed&' . $identifier) . '" width=":w" height=":h" frameborder="0" allowfullscreen="allowfullscreen" title="' . $title . '"></iframe>',
      'resizeCode' => '<script src="' . plugins_url('h5p/h5p-php-library/js/h5p-resizer.js') . '" charset="UTF-8"></script>',
-      'url' => admin_url('admin-ajax.php?action=h5p_embed&id=' . $content['id']),
+      'url' => admin_url('admin-ajax.php?action=h5p_embed&' . $identifier),


I think $identifier should also run through esc_attr(), see above.

otacke · 2026-08-03T09:20:21Z

@tomekstach Hope you're still around and interested?

tomekstach · 2026-08-03T11:58:32Z

@otacke
Hi,
Yes, I am definitely interested. Unfortunately, my workload has been so heavy that I haven't had a chance yet to review your suggestions in the code. I will do my best to go through everything this week and implement the necessary changes.

Best regards,

otacke · 2026-08-03T12:15:43Z

@tomekstach Thanks for your reply! And there's no reason to hurry, really. This has been dormant for years now - sorry for that again.

I'm just following up to learn whether pull requests could be closed meanwhile, because the contributors lost interest. As someone with 100+ open pull requests myself, some dating back to the last decade, I could fully understand.

I'll keep the pull request in the "waiting for contributor" column for now then (H5P Open Source Project). Once you're done, this will go to "verified by community maintainer", and then the ball is in H5P Group's court eventually. They hopefully won't require years to empty that column ...

otacke · 2026-08-28T10:56:08Z

@tomekstach I don't want to push, just set a reminding "ping" after 4 weeks.

tomekstach · 2026-08-31T15:51:24Z

@otacke please, review my changes.

otacke · 2026-08-31T16:41:15Z

Just that one comment about characters used in slugs ...

otacke

Excellent! I'll move the request to the status "verified by community maintainer" (see https://github.com/orgs/h5p/projects/6/views/7), meaning the pull request is to be picked up by H5P Group once it has been prioritized. Please note, however, that I don't know when they are going to handle this. My powers end here.

otacke · 2026-08-31T18:38:50Z

Info for H5P Group

This pull request implements the user story:

"As as admin/author, when I want to embed H5P content, I want to be able to use the content slug instead of the content id"

tomekstach · 2026-08-31T20:10:08Z

@otacke Thank you so much!

Enable the use a slug in embed

3f8fd7e

tomekstach added 2 commits September 24, 2025 10:47

Merge branch 'master' into add-slug-to-embed

996f42b

Merge branch 'master' into add-slug-to-embed

d9fa94a

ghapp-ghproj Bot added this to H5P Open Source Project Mar 23, 2026

github-project-automation Bot moved this to Todo in H5P Open Source Project Mar 23, 2026

otacke suggested changes Jul 28, 2026

View reviewed changes

github-project-automation Bot moved this from Backlog to Waiting for Contributor in H5P Open Source Project Jul 28, 2026

otacke added the feature New or improved product feature label Aug 7, 2026

Applied otacke's suggestions

0fafdad

Fix for the slug sanitization

d812df2

otacke approved these changes Aug 31, 2026

View reviewed changes

otacke moved this from Waiting for Contributor to Verified by Community Maintainer in H5P Open Source Project Aug 31, 2026

Uh oh!

Conversation

tomekstach commented Jul 7, 2022

Uh oh!

bernhardkaindl commented Aug 6, 2023

Uh oh!

otacke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

otacke commented Aug 3, 2026

Uh oh!

tomekstach commented Aug 3, 2026

Uh oh!

otacke commented Aug 3, 2026

Uh oh!

otacke commented Aug 28, 2026

Uh oh!

tomekstach commented Aug 31, 2026

Uh oh!

otacke commented Aug 31, 2026

Uh oh!

otacke left a comment

Choose a reason for hiding this comment

Uh oh!

otacke commented Aug 31, 2026

Info for H5P Group

Uh oh!

tomekstach commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants