Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

Usage of the iframe false positive? #143

Open
danyj opened this issue Mar 13, 2019 · 5 comments
Open

Usage of the iframe false positive? #143

danyj opened this issue Mar 13, 2019 · 5 comments
Labels
Status: Awaiting Feedback The issue or fix needs more feedback from people with more experience about it. Status: Second Opinion Second opinion is needed on the issue/enhancement. Type: Question This is a question regarding the functionality of the plugin.

Comments

@danyj
Copy link

danyj commented Mar 13, 2019

Is this false positive or?
It says Usage of the iframe HTML element is prohibited

This is the line that triggers it
https://github.com/Themezly/Creatus/blob/WP-Org-Prep/assets/js/thz.site.js#L3114

but the iframe element is needed for magnific popup to display all video sources.

https://github.com/dimsemenov/Magnific-Popup/blob/master/src/js/iframe.js#L27

@timelsass
Copy link
Member

timelsass commented Mar 14, 2019

The iframe check happens upstream in the repository WPTRT/WPThemeReview so any bugs should be reported there. I believe this check is stating that themes are not responsible for content, in which case the code would best be added to a plugin that provides that functionality.

@dingo-d
Copy link
Member

dingo-d commented Mar 23, 2019

As far as I'm aware we are not allowing iframes in themes. This was added in the old theme check plugin, so I'm not sure the theme would pass the upload phase if it had it.

https://make.wordpress.org/themes/handbook/review/required/theme-check-plugin/

@joyously @carolinan @jocastaneda @justintadlock comments?

@dingo-d dingo-d added Status: Second Opinion Second opinion is needed on the issue/enhancement. Type: Question This is a question regarding the functionality of the plugin. Status: Awaiting Feedback The issue or fix needs more feedback from people with more experience about it. labels Mar 30, 2019
@pattonwebz
Copy link
Member

Themes have not been allowed to add iframes in the past. This is mostly to prevent them loading in external content that could be dangerous. Exceptions might be made in certain situations but as a general rule iframes are not allowed.

If the videos loaded by this theme into the iframe are urls that the user has input themselves then this might be one of the exceptions to the general rule but I really don't see any way of us sniffing around it.

The sniff should probably still flag all instances of iframe for farther inspection.

@danyj
Copy link
Author

danyj commented Mar 31, 2019

Just so we are clear , we are talking about ligtboxes here like tb_iframe, not an inclusion of iframe or anything else. Here
https://github.com/brainstormforce/astra/blob/35c7845e566ae6c4c619ad8a5abd4badd589d315/inc/core/class-astra-admin-settings.php#L293

and same scripts Magnific Popup and Fitvid are already in the themes repo.

https://github.com/oceanwp/oceanwp/blob/master/assets/js/third/magnific-popup.js#L1572
https://github.com/oceanwp/oceanwp/blob/0ea55c592fc757366083140b9739539f9d870b77/assets/js/devs/fitvids.js#L36

There is no other way for lightbox to display any 3rd party service video without it.

@pattonwebz
Copy link
Member

@danyj I am aware of the kinds frames this Issue was opened about and do understand how they differ from the normal kinds of 'iframes are not allowed' that the rule is for. That is why I said that these kinds of things may be an exception to the general rule (also noting that I believe it would be a trouble to discount these kinds of frames in the sniff).

I was just adding some notes to this issue for future reference - sorry if I was unclear about that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Awaiting Feedback The issue or fix needs more feedback from people with more experience about it. Status: Second Opinion Second opinion is needed on the issue/enhancement. Type: Question This is a question regarding the functionality of the plugin.
Projects
None yet
Development

No branches or pull requests

4 participants