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

Preview iframe system in "down" popup needs deep review #49

Open
jerclarke opened this issue Jan 25, 2018 · 2 comments
Open

Preview iframe system in "down" popup needs deep review #49

jerclarke opened this issue Jan 25, 2018 · 2 comments

Comments

@jerclarke
Copy link

jerclarke commented Jan 25, 2018

NOTE: This is related to another bug we are still identifying, that breaks the JS-based admin-ajax.php calls that help render this box, and if we can fix that bug, the box should have more messages in it if nothing else. It seems to apply to Nginx in particular and we are going to propose a separate patch.

When a link is down, javascript makes a popup show when the link is clicked. It says the link isn't working and offers "View the Snapshot".

This is great, it's the essence of Amber.

But why does this include an iframe that loads the original URL?

amber-empty-iframe-preview

This white box is exactly what we'd expect, since most modern sites will not allow arbitrary loading of their content in an iframe.

Overall, this seems like both bad usability, since it's usually an unlabeled, useless white box, and a security issue, because we are loading an iframe with what could be a compromised site.

What is the purpose of this box and is it necessary? Could it be made to load something more useful, or to only display if there is content to show?

@ryanttb
Copy link
Collaborator

ryanttb commented Jan 25, 2018

Hi Jer, the white box is supposed to show the page you're trying to get to as a live preview so the user can better determine if it's really down or if they should try an archive. The iframe is sandboxed so security should be ok. It's true that sites are blocking iframes but the JS adding the preview gets no indication/events on whether or not the external site loaded ok.

Agreed that some context labels would be helpful here. Happy to hear other suggestions, too!

@jerclarke
Copy link
Author

Okay so the biggest hindrance to this for us was the bug fixed by this PR: #50

It was making the popup totally misbehave on our Nginx setup.

@jlicht jlicht mentioned this issue Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants