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

Web preview overlay hides iframe errors #260

Closed
jankeromnes opened this issue Jan 23, 2019 · 9 comments
Closed

Web preview overlay hides iframe errors #260

jankeromnes opened this issue Jan 23, 2019 · 9 comments
Assignees
Labels
type: bug Something isn't working

Comments

@jankeromnes
Copy link
Contributor

Describe the bug

When a web preview fails because of some error, an overlay remains visible in the Preview pane, hiding the actual error.

To Reproduce

  1. Open this snapshot in Gitpod:

Open in Gitpod

  1. A server will start on port 4242, and open a Preview pane, but unfortunately it stays grey

Expected behavior

The Preview should show the iframe error, instead of the unhelpful grey overlay.

Work around

You can work around this bug by manually deleting the overlay using the browser devtools:

screenshot 2019-01-23 at 17 59 42

screenshot 2019-01-23 at 18 00 17

screenshot 2019-01-23 at 18 00 33

Now you can see the actual error that the iframe encountered, and try to address it.

@jankeromnes jankeromnes added the type: bug Something isn't working label Jan 23, 2019
@svenefftinge
Copy link
Member

cc @kittaakos

@kittaakos
Copy link

Related? eclipse-theia/theia#3589 (comment)

@jankeromnes
Copy link
Contributor Author

Well, the CSP error is common, but this bug is about Gitpod hiding any iframe error behind an overlay, while that doesn't seem to happen in Theia itself.

@jankeromnes
Copy link
Contributor Author

The overlay that remains visible (hiding iframe errors), only in Gitpod but not in stock Theia, is:

<div class="theia-mini-browser-load-indicator" style="display: block;"></div>

I'll tentatively look into fixing this, but I don't yet know Theia or its Gitpod Extension very well, so I might end up deferring to you @kittaakos or @AlexTugarev.

@jankeromnes jankeromnes self-assigned this Feb 18, 2019
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Feb 20, 2019

Reading the code, it looks like neither onFrameLoad() nor catch (e) are being called in the case of iframe errors (it's the only two relevant locations that can call this.hideLoadIndicator()). Maybe things like CSP errors get caught somewhere else and not forwarded?

EDIT: Likely exception eater is this catch (would make cross-origin security exception disappear). Maybe it should forward the exceptions instead of hiding them?

EDIT 2: Actually, that code runs after a 'load' event, which never happens after a CORS error.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Feb 20, 2019

Here are the various iframe error behaviors in my two browsers:

In Firefox (Nightly)

  • gp preview https://www.google.com
    • Console error:Load denied by X-Frame-Options: https://www.google.com/ does not permit cross-origin framing.
    • 'Load' event: ✅ (loading indicator disappears)
    • 'Error' event:
    • Iframe content: ❌ (blank)
  • gp preview http://perdu.com
    • Console error:Blocked loading mixed active content “http://perdu.com/”
    • 'Load' event: ❌ (loading indicator hides iframe)
    • 'Error' event:
    • Iframe content: ❌ (blank)
  • gp preview https://github.com
    • Console error: ❌ (none)
    • 'Load' event: ❌ (loading indicator hides iframe)
    • 'Error' event:
    • Iframe content:Blocked by Content Security Policy. An error occurred during a connection to github.com.
  • gp preview https://www.asdfasdf.asdf
    • Console error: ❌ (none)
    • 'Load' event: ❌ (loading indicator hides iframe)
    • 'Error' event:
    • Iframe content:We can’t connect to the server at www.asdfasdf.asdf.

In Chrome

  • gp preview https://www.google.com
    • Console error:Refused to display 'https://www.google.com/' in a frame because it set 'X-Frame-Options' to 'sameorigin'.
    • 'Load'` event: ✅ (loading indicator disappears)
    • 'Error' event:
    • Iframe content: ✅ Sad file icon + www.google.com refused to connect. on hover
  • gp preview http://perdu.com
    • Console error:Mixed Content: The page at 'https://d42381f3-70bb-4bf0-8099-9b05c27c3d25.ws-eu0.gitpod.io/#/workspace/theia' was loaded over HTTPS, but requested an insecure resource 'http://perdu.com/'. This request has been blocked; the content must be served over HTTPS.
    • 'Load' event: ❌ (loading indicator hides iframe)
    • 'Error' event:
    • Iframe content: ❌ (blank)
  • gp preview https://github.com
    • Console error:Refused to display 'https://github.com/' in a frame because an ancestor violates the following Content Security Policy directive: "frame-ancestors 'none'".
    • 'Load' event: ✅ (loading indicator disappears)
    • 'Error' event:
    • Iframe content: ❌ (blank)
  • gp preview https://www.asdfasdf.asdf
    • Console error: ❌ (none)
    • 'Load'` event: ✅ (loading indicator disappears)
    • 'Error' event:
    • Iframe content: ✅ Sad file icon + www.asdfasdf.asdf’s server IP address could not be found. on hover

@jankeromnes
Copy link
Contributor Author

In summary, the loading indicator overlay currently hides these iframe errors:

  • In Firefox:
    • Mixed Content (but iframe is blank)
    • CSP error
    • Can't connect
  • In Chrome:
    • Mixed Content (but iframe is blank)

If we remove the loading indicator overlay (either completely, or hide it after a timeout) we'll reveal the above iframe error (except Mixed Content, where the iframe remains blank).

@jankeromnes
Copy link
Contributor Author

Aha, it seems that, in the case of a Mixed Content error, iframe.addEventListener('error', event=>{ }) does yield something. Maybe we can subscribe to error events, and display something useful when they happen.

@jankeromnes
Copy link
Contributor Author

Fixed in eclipse-theia/theia#4399 and deployed. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants