Make InstantClick work on more sites without configuration #94
Conversation
Some sites such as Dropbox.com use attributes on script nodes to define how they should be ran (or include non-js script nodes). Previously, only the innerHTML and src attributes would be preserved, often triggering errors or broken functionality. This change uses the cloneNode function to copy the node while preserving all attributes. cloneNode is not supported by FF before 13, so if legacy support is desired an alternate solution will have to be found.
Previously assets in the head of the preloaded page wouldn't be loaded, as only the body was copied. After this change, any tag in the head of the previous page, not in this page, is added to the head after the switch.
| function changePage(doc, newUrl, scrollY) { | ||
| updateHeadResources(doc.head) | ||
| updateAttributes(doc.head, document.head) | ||
| updateAttributes(doc.documentElement, document.documentElement) |
There was a problem hiding this comment.
updateAttributes allows us to propagate changes to the html or head elements attributes (i.e. classes) to the new page.
… not a boolean parameter
If someone has called `e.preventDefault()`, or returned false from an event handler in IE8, respect that cancellation.
|
Wow, great changes! |
… again. Don't remove attributes to not break Typekit and the like.
… again. Don't remove attributes to not break Typekit and the like.
…reexecuted Conflicts: instantclick.js
Conflicts: instantclick.js
|
Nice work. |
|
I have a million other things to do so I haven’t reviewed that yet, but if it’s as worthwhile as it sounds it’s very much appreciated! |
… rerunning scripts that call it
…the scripts twice.
|
Sorry for making you wait so long, that’s a pretty big list of changes to review (and I’ve haven’t reviewed them all). Adding stylesheets in the A mechanism to prevent that would add a lot of complexity, in my experience once you fight the browser’s default behaviors with JavaScript you then need an enormous amount of code to make it work well, so I prefer to keep InstantClick’s battles against the browser’s behaviors to a minimum. So unfortunately, it looks like InstantClick won’t ever be “plug and play”. Thanks for making your fork of InstantClick progress though. There’s certainly good stuff to pick in there. I’ll check other commits in this PR a little bit at a time in the future, if there are things that catch my attention I’ll let you know and we’ll see to incorporate them into InstantClick. |
The unpleasant flash is called a flash of unstyled content. I just wanted to say that in case anybody didn't know. |
The goal of these changes is to make IC work on more sites without requiring the addition of
data-attribute configuration. Before these changes, it was common for sites which had changes in their head to break after IC navigation. I tried to make the commit messages descriptive, so please consult them for a specific list of the changes required.Here's a few examples of sites broken w/ master which now work:
The one note about these changes is they don't target Firefox before version 13. This amounts to about 0.2% of the web browser market. If this is an issue, let me know and I will tweak the solutions to work on those older browsers as well.