-
Notifications
You must be signed in to change notification settings - Fork 524
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
fix(livesamples): use correct legacy url #9158
Conversation
We did not use the legacy URL in LiveSampleURL.ejs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but while you're cleaning up:
kumascript/macros/LiveSampleURL.ejs
Outdated
|
||
if ($1 && $1.length > 0) { | ||
mdn.deprecatedParams("The second parameter of 'LiveSampleURL' is deprecated"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, the message would have an actionable suggestion on how to proceed. I guess in this case the original example has to be copied into the page, right?
kumascript/macros/LiveSampleURL.ejs
Outdated
var pagePath = env.url; | ||
let pagePath = env.url; | ||
let baseUrl = env.live_samples.base_url; | ||
let samplePath = `${pagePath}/runner.html`.replace(/\/\/+/g, "/"); | ||
|
||
if ($1 && $1.length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're cleaning up the macro anyways, can you assign $0 and $1 to variables id
and url
or similar to avoid referencing $0 and $1 below?
Summary
We did not use the legacy URL in LiveSampleURL.ejs
Problem
Live samples translated from another page (deprecated), resulted in to 404 iframe.
Solution
Use the legacy url in LiveSampleURL.ejs to point to the legacy live sample.
Screenshots
Before
After
How did you test this change?
https://developer.mozilla.org/en-US/docs/Learn/Forms/How_to_build_custom_form_controls
With:
Use a
yarn build files/en-us/learn/forms/how_to_build_custom_form_controls/index.md
and check that the iframes in the resulting html file havelive-samples.mdn.mozilla.net
as domain if they are not using therunner.html
.