Make YoutubeLinkEnhancement URL parsing more robust. #4003
Conversation
sengi
force-pushed
the
sengi/parse-urls-properly
branch
from
8346d0e
to
dcb2e38
Compare
sengi
force-pushed
the
sengi/parse-urls-properly
branch
from
dcb2e38
to
4621243
Compare
[URL API] `searchParams` works in all browsers as of 2017. It also considerably simplifies the code and makes the intent much clearer. Resolves https://github.com/alphagov/govuk_publishing_components/security/code-scanning/1 [URL API]: https://developer.mozilla.org/en-US/docs/Web/API/URL
sengi
force-pushed
the
sengi/parse-urls-properly
branch
from
4621243
to
c8ecaa4
Compare
There was a problem hiding this comment.
@sengi looks like a sensible change except that includes and (haven't confirmed this yet but fairly sure) searchParams aren't supported by older browsers like IE. While we don't support those browsers we do want to ensure they have a reasonable experience of GOV.UK and this change would probably cause all of the JS to break suddenly.
We're currently working on an RFC to remove JS support for IE11, can this PR wait until then? alphagov/govuk-rfcs#171
|
Ah ok, TIL we still de facto support IE11 😅 If you're sure we need to continue to support embedded YT videos in IE11 for the time being then I'm happy to stick this on the back burner until you're ready to drop IE. I'll make it draft for now anyway. |
|
Do we have a preferred polyfill library for this sort of thing? (Not necessarily suggesting it's worth it, just curious.) |
|
Ah no I was grepping wrong; it is at least pulled into some of the public frontends. |
|
Even so, I'm not sure this would affect anything public-facing because it's in the govspeak library and surely we're not doing the govspeak->html transform on the client? 🤔 |
Correct, we're not doing the govspeak->html transform on the client, but we are running this JS on that output in the client to convert links to videos into embedded videos. See this page for an example - if JS is disabled the embedded videos show simply as links to Youtube. |
|
Aha thanks! Super helpful example ❤️ |
URL API
searchParamsworks pretty much everywhere since around 2017, which includes all the user agents we support. It also considerably simplifies the code and makes the intent much clearer.(We have to avoid static
canParse()though, because Samsung Internet is weird.)Resolves https://github.com/alphagov/govuk_publishing_components/security/code-scanning/1.
No visual or functional change intended.