Problem/Motivation

While using drupal with decoupled application with NextJs, embedded media gives 404 error.
This is because the iframe source provided by media oembed is /media/oembed?url=.

Steps to reproduce

  1. Install Drupal any version mine is 9.4.x
  2. Install NextJs module
  3. Setup NextJs site
  4. Create node with embedded video
  5. Visit page in NextJs site, it will give you 404 for the video embedded section

Proposed resolution

Provide option in media settings form(i.e. /admin/config/media/media-settings) to use absolute path for embedded media(may be checkbox, as provided for standalone url).
Output the full path as absolute.

Remaining tasks

  1. Provide checkbox on Media settings form


  2. Update OEmbedFormatter

    3.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 media-absolute-oembed-path-3311469-14.patch648 bytesinternetter
Screen Shot 2022-09-23 at 6.37.24 PM.png212.57 KBrajeshreeputra

Issue fork drupal-3311469

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Rajeshreeputra created an issue. See original summary.

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Issue summary: View changes
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Assigned: Unassigned » rajeshreeputra
Issue summary: View changes
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Assigned: rajeshreeputra » Unassigned
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Active » Needs review

Rajeshreeputra opened merge request !2795

Rajeshreeputra closed merge request !2795

Rajeshreeputra opened merge request !2796

heni_deepak’s picture

heni_deepak commented

@Rajeshreeputra
can we use it without NextJs work dependency? I think NextJs is out of Drupal core.

cilefen’s picture

cilefen commented
Version: 9.4.x-dev » 10.1.x-dev
Category: Bug report » Feature request
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

@heni_deepak, You can reproduce this with headless Drupal with different domains(DNS).
Drupal as backend on dns1.com and different frontend with dns2.com.
Now create article or any other content type node with embedded video and visit your frontend site you will see the 404 for embedded video iframe section.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Absolute path for oembed ifram » Absolute path for oembed iframe
Status: Needs review » Postponed (maintainer needs more info)

Isn't this just a bug in the decoupled application? It's the responsibility of any decoupled application to transform relative URLs?

trevorkjorlien’s picture

trevorkjorlien commented
Related issues: +#3332371: Attempt to adapt Remote oEmbed videos so they appear in Safari/Firefox 'reader' mode
internetter’s picture

internetter commented
StatusFileSize
new media-absolute-oembed-path-3311469-14.patch648 bytes

If somebody only needs the change to absolute oembed path. We made only this change, see patch attached.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

skyredwang’s picture

skyredwang
Location Shanghai
commented
Status: Postponed (maintainer needs more info) » Active

> Isn't this just a bug in the decoupled application? It's the responsibility of any decoupled application to transform relative URLs?

No, I think this is at least a DX issue. When CKeditor is used to embed a remote media, the iframe is embedded in the body of a long text. For most front end applications, sure, they can add a domain to the URL that output by Media OEmbed, but that means the front end needs to parse the body of the text.

For few frontend apps, multiple Drupal backends can be connected at the same time, therefore, the frontend would have a little bit more logic to write when prefixing a domain.

Proposed solution: And a checkbox somewhere to allow Media OEmbed to output absolute URLs when checked, and default to relative path for backward compatibility

skyredwang’s picture

skyredwang
Location Shanghai
commented
Version: 11.x-dev » 10.1.x-dev
fjgarlin’s picture

fjgarlin commented

I could reproduce this situation when testing Drupal in a subdirectory (ie:https://localhost/subdirectory).

We are trying to convert DrupalCI to GitlabCI in this issue: #3375827: Investigate tests failures

One of the errors we were getting was in "MediaSourceOEmbedVideoTest"
* Job: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/j...
* Error: ElementNotFoundException: Element matching css "iframe.media-oembed-content" not found.

Upon investigation, this was due to having "/subdirectory/media/oembed?..." in the src attribute (note the starting "/"), but when prepending the base path, then "https://localhost/subdirectory/subdirectory/media/oembed?..." doesn't exist.

The fix:
* Pretty much the same as #14 (setting the value of the URL to absolute).
* See the second change here: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/c...
* Job: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/j... . See how the "MediaSourceOEmbedVideoTest" is gone.

I think #14 is the cleaner and easier solution, with no UI changes needed, but the suggested MR would also solve the issue.

fjgarlin’s picture

fjgarlin commented
Status: Active » Needs review

See my previous comment for further context as this seems to be an issue as well in installations running in a subdirectory.

I've going to set this issue as "Need review" with the suggested fix in #14.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests

Believe the issue summary should be updated if the solution for #14 is the path forward. Will also needed test coverage.

catch’s picture

catch
he/him
Primary language English
commented
Category: Feature request » Bug report
Priority: Normal » Critical

This is blocking gitlab CI.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes

Updated slightly the issue description to match the suggested fix.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
Issue tags: -Needs tests +Needs followup

Given this fails on gitlab ci that will be plenty of implicit test coverage so happy to add explicit coverage in a follow-up.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs followup

Opened #3384881: Add tests for oembed iframe absolute

  • catch committed 05e455ed on 11.x 
    Issue #3311469 by Rajeshreeputra, Internetter, fjgarlin, skyredwang:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 05e455e and pushed to 11.x. Thanks!

  • catch committed 80b264b1 on 10.1.x 
    Issue #3311469 by Rajeshreeputra, Internetter, fjgarlin, skyredwang:...
quietone’s picture

quietone commented
Issue tags: -Needs issue summary update

The issue summary was updated a bit in #22

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.