mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.19k stars 508 forks source link

Add feature request property option to github live sample macro #4354

Open Rumyra opened 4 years ago

Rumyra commented 4 years ago

Summary As it stands any Web APIs that require feature request properties to run in an iframe can not use the github live sample feature to add demos to pages, as the generated containing iframe does not have the correct attributes to allow demos to work. Adding the ability to add the necessary attributes would then allow for code examples/demos to use github embeds (which is a really nice feature)

Audience Article authors. I am currently experiencing this issue, and with the 'rise' of more APIs which harness feature request I predict it will become a more wanted feature among authors.

Rationale Github embeds are great because the code is held within the MDN eco system. Having used codepen embeds before (which is also a great feature) the onus is on myself and my personal codepen account to maintain such demos. Whilst this isn't a huge issue, having the code within the MDN organisation automatically allows open source maintenance which I believe helps manage ever changing APIs and related demos.

Workaround A link to code rather than an embed. For the end user it's nice to see a working demo.

Proposal Implement feature policy on embedded iframes. Top priority is the security risks of doing so on an open source wiki. (I have no further information on this at this time however). Update the github embed macro to include an optional comma separated list of wanted feature policies, so for example:

{{EmbedGHLiveSample("guthub/url/to/example", 'width', 'height', 'feature, policies, as, a, list')}}

This functionality could also be extended to be used with traditional live examples as well.

Additional context My example is here: https://wiki.developer.mozilla.org/en-US/docs/Web/API/Screen_Wake_Lock_API#Putting_it_all_together

I've had to link to the code rather than embed it (which is preferable) as the demo can't work in a iframe with 'screen-wake-lock'

EDIT Another example: Fullscreen as per this issue https://github.com/mdn/sprints/issues/3828

tobinmori commented 4 years ago

Evaluate security implications and determine path forward (whether worth doing).

peterbe commented 3 years ago

@Rumyra Can you reassess this and determine if we should close it or transfer it to Yari?

Rumyra commented 3 years ago

@peterbe I did a lot of thinking... yes this is useful. However probably not a p1 at the moment with everything else happening. So can we move it to Yari with a p2 to be addressed 'at some point'?

schalkneethling commented 2 years ago

@peterbe I did a lot of thinking... yes this is useful. However probably not a p1 at the moment with everything else happening. So can we move it to Yari with a p2 to be addressed 'at some point'?

Hey @Rumyra, have your thoughts on this changed at all? A higher priority now, or maybe lower? Thanks!

Rumyra commented 2 years ago

I think until we get around to addressing all the many API demo repos we have this won't creep up in priority. This is still very much on my bucket list, as we need to start prioritising these repos. However if it's in your way @schalkneethling I'm happy for you to close it and I can re-open it when I inevitably come across this issue again 👍