- 🇦🇺Australia sime Melbourne
I decided to work on a template site and clean up some warnings in the status report. I've never really focussed on this warning before and I can point to commodity government platforms that do not set this setting by default - I guess they made an assessment about the risk and decided to live with it.
Anyway here's just an anecdotal story about trying to remove this error on Platform.sh, and might be a good benchmark to consider the UX for non-technical people.
I figured the goal was to add another domain to my site. Like a multi-site I guess. Platform.sh allows you to add multiple domains to point to the same site, but the way all the examples are setup they assume you'll be redirecting to one domain at the edge. They have an option to handle all domains as legitimate (
"https://{all}/"
), but then you're faced with what to do with this - this functionality was likely added to support a traditional multisite setup with multiple legitimate domains.I guess it wasn't too hard to set it up on Platform.sh after I decided how to set up my routes, but there was definitely not a way to generically set it up. Drupal 10 doesn't really have a domain setting since we removed
$base_url
, which is something I take for granted when setting up a boilerplate site. This setting appears to require a fully qualified domain, so I'm either faced with hardcoding this into the settings, or programmatically set it in settings.php. I guess the mum/dad site builders will just hardcode it once they work out how to add another domain to their site.For the sake of sharing my thinking patterns this is what i settled on. I decided to base the media domain on the main domain. I wanted to make this a subdomain like
media.demo-site
but then I had to negotiate certificate setup on Cloudflare. If was using the apex this would have been a little easier i could have used www. and non-www. for eg.So then i have two domains. I really don't want them both to serve the main site. Redirect module doesn't provide any handling for this case in the global redirect handling - how could it, it doesn't know the base url since Drupal 8. I guess I'm going to code this redirect. I don't think this is a great option for site builders at all and it feels weird that Drupal is making me do all this to get rid of a warning that I know for a fact a lot of sites never address.
Am i missing something in how to address this issue?
- Assigned to ivnish
- Merge request !9074Resolve #2962753 "Expose a way to suppress oEmbed security warnings" → (Open) created by ivnish
- Issue was unassigned.
- Status changed to Needs review
6 months ago 10:38am 5 August 2024 - Status changed to Needs work
6 months ago 2:58pm 5 August 2024 - Status changed to Needs review
6 months ago 8:40am 8 August 2024 - Status changed to Needs work
5 months ago 3:06pm 11 August 2024 - 🇺🇸United States smustgrave
This one feel close
So the issue summary does need to be complete. Currently has TBD sections, proposed solution needs to be filled in. UI Changes should have a screenshot in that section, API and Data model probably N/A here.
Also the new setting will require a change record so tagging for that.
Thanks.
- Status changed to Needs review
5 months ago 7:10am 12 August 2024 - ivnish Kazakhstan
Issue summary fixed
New change record was created https://www.drupal.org/node/3467521 →
- Status changed to RTBC
5 months ago 12:51pm 12 August 2024 - 🇳🇿New Zealand quietone
I read the is, comment and the MR. Everything is in order here except there should be screen shots available from the issue summary.
I also think the red line in the 'after' screenshot in the change record should be removed and something like a diagonal arrow used to direct attention to the addition. But I am not setting to needs work for that small change. I am assuming the reader will understand.
- 🇬🇧United Kingdom longwave UK
I'm not really sure we should be adding a setting for this, especially given 📌 [policy, no patch] Secondary subdomain for viewing oEmbed content is confusing and pointless Active which might remove the feature entirely. Postponing this and will try to reactivate the discussion over there.
- 🇧🇾Belarus krakenbite
which might remove the feature entirely.
The decision to remove it will take several years, but you can remove the annoying banner right now
- Status changed to Active
2 months ago 4:21am 22 November 2024 - 🇦🇺Australia pameeela
Based on #3208830-27: [policy, no patch] Secondary subdomain for viewing oEmbed content is confusing and pointless → , the first agreed step is to remove this warning altogether. So let's do that!
- 🇦🇺Australia pameeela
Oh, I didn't notice there was already an MR for the setting.
- ivnish Kazakhstan
@pameeela I added a second MR with removing this warning
Hi,
Test status - Pass
Tested on Drupal Version - 11.X
Steps to reproduce -1 Enable Media module
2 Go to Drupal status pageOembed warning is no longer visible.
Please refer the screenshots.- 🇮🇳India sagarmohite0031
Hello,
Verified and Tested on Drupal 11,
Patch applied successfully.Steps to reproduce -
1-Enable Media module
2-Go to Drupal status pageWarning is no longer visible.
attaching screenshots
RTBC+1 - 🇬🇧United Kingdom longwave UK
There is test coverage for this message which also needs removing.
Drupal\Tests\media\Functional\MediaSettingsTest::testStatusPage Behat\Mink\Exception\ResponseTextException: The text "It is potentially insecure to display oEmbed content in a frame" was not found anywhere in the text of the current page.
- 🇬🇧United Kingdom longwave UK
longwave → changed the visibility of the branch 2962753-expose-a-way to hidden.
- 🇨🇦Canada joelpittet Vancouver
Back to RTBC, thanks for removing the test as well ivnish
-
longwave →
committed 416a7796 on 10.4.x authored by
lauriii →
Issue #2962753 by ivnish, asawari, sagarmohite0031, phenaproxima,...
-
longwave →
committed 416a7796 on 10.4.x authored by
lauriii →
-
longwave →
committed 697318c5 on 10.5.x authored by
lauriii →
Issue #2962753 by ivnish, asawari, sagarmohite0031, phenaproxima,...
-
longwave →
committed 697318c5 on 10.5.x authored by
lauriii →
-
longwave →
committed b04a9465 on 11.1.x authored by
lauriii →
Issue #2962753 by ivnish, asawari, sagarmohite0031, phenaproxima,...
-
longwave →
committed b04a9465 on 11.1.x authored by
lauriii →
- 🇬🇧United Kingdom longwave UK
Discussed with the release managers, additionally backported this to 11.1.x and 10.4.x/10.5.x so it can get into the forthcoming releases.
Automatically closed - issue fixed for 2 weeks with no activity.