search

chriszarate / supergenpass

A free bookmarklet password generator.
https://chriszarate.github.io/supergenpass/
GNU General Public License v2.0
418 stars 160 forks source link

#46 popup on error, rather than redirect #48

Closed cmcnulty closed 10 years ago

cmcnulty commented 10 years ago

My solution for #46, does a popup on error of the script load, and always attempts a script injection as a method of trapping script injection blocking. We do this on the assumption that if a site is blocking script injection, they're also blocking frame injection, and we can trap for script injection, but in order to trap for it, we can't skip loading the script in the case that the site is already hosting jQuery.

cmcnulty commented 10 years ago

Line 114 should also adjust the size of the pop-up when the content is resized:

popup.resizeTo(x, y);
chriszarate commented 10 years ago

Thanks for all the thoughts and work on this. It's a huge bummer that we can't trap the CSP iframe error—I gather that there's some privacy issues that this would expose, and that may be why the browsers don't fire an event.

Your approach is great, but loading jQuery every time and assuming that a script loading error is a predictor for the CSP error still felt shaky. After some thought, I think I've settled on a slightly different approach:

  1. Properly catch the script loading error, if it happens.
  2. Add a loading message to the SGP box while the iframe loads.
  3. If the iframe loads, remove the loading message.
  4. If after a timeout the iframe is not there, add a message and a link to the mobile version.

Thoughts?

cmcnulty commented 10 years ago

Definite improvement over current behavior, and I agree whole-heartedly that my "solution" was making tenuous assumptions, at best. I do think that SGP in a pop-up could be a good solution, but ideally, you'd still want some cross-window communication. Providing a link, which allows users to open it in a new tab or navigate the current window is a good solution, and provides the end user with the most control, which is always a good thing in my mind.

chriszarate commented 10 years ago

Ok, take a look. I ended up hooking into the postMessage cross-window communication to determine if it had loaded or not. I don't there's any other way to tell. Definitely room for improvement, both functionally and aesthetically, but seems to work.