Skip to content
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

Add support for IE 11 for markup slices #3702

Merged
merged 2 commits into from
Oct 23, 2017
Merged

Add support for IE 11 for markup slices #3702

merged 2 commits into from
Oct 23, 2017

Conversation

jaylindquist
Copy link

Because srcdoc-polyfill uses a javascript: URL for the src attribute, the allow-scripts and allow-same-origin sandbox options need to be set, otherwise Internet Explorer will not be able to execute the script. This technically breaks all sandboxing as any javascript within the iframe can now manipulate the iframe element itself as well as any other element on the page.

Added allow-top-navigation to support links in the markup slice with target="_top"

Added allow-popups to support links in the markup slice with target="_blank"

…ces. Add allow-top-navigation and allow-popups to support links within iframes
@coveralls
Copy link

coveralls commented Oct 21, 2017

Coverage Status

Coverage remained the same at 70.757% when pulling 727ce58 on jaylindquist:jay/ie-iframe into e121a85 on apache:master.

@@ -23,9 +24,11 @@ function markupWidget(slice, payload) {
<iframe id="${iframeId}"
frameborder="0"
height="${slice.height()}"
sandbox="allow-scripts">
sandbox="allow-same-origin allow-scripts allow-top-navigation allow-popups">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the allow-top-navigation allow-popups part?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are to fix #3672. When the markup slice was moved to use iframe, any link inside the slice would only open within the slice and links meant to be opened in another tab/window did nothing.

We use this for documentation and referencing other sites.

  • allow-top-navigation lets users create a link with target="_top" that will open the link in the same tab
  • allow-popups lets users create a link with target="_blank" that will open the link in a new tab

Without these two fields links will only open inside the iframe itself

</iframe>`);
$('#' + iframeId)[0].srcdoc = html;

const iframe = $('#' + iframeId)[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is off and while we are at it we may just use a plain dom js function instead of jquery

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jquery is now removed

@coveralls
Copy link

coveralls commented Oct 23, 2017

Coverage Status

Coverage remained the same at 70.757% when pulling d0238f2 on jaylindquist:jay/ie-iframe into e121a85 on apache:master.

@mistercrunch mistercrunch merged commit b4bdc45 into apache:master Oct 23, 2017
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Add srcdoc-polyfill tosupport Internet Explorer iframes in markup slices. Add allow-top-navigation and allow-popups to support links within iframes

* Remove jquery from markup.js
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.5 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants