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

Turbolinks should follow same-page anchor links without reloading the page #75

Open
preetpalS opened this Issue Apr 15, 2016 · 28 comments

Comments

Projects
None yet
@preetpalS

preetpalS commented Apr 15, 2016

Consider the following HTML link to an element with an id of bookmark:

<a href="#bookmark" >Bookmark within page</a>

Turbolinks currently intercepts this link and prevents the browser from following it unless you add an annotation on the link (data-turbolinks="false"). Once Turbolinks has intercepted the link, it makes another HTTP request for the same location again (but with the id appended at the end (/url#bookmark)) instead of just scrolling to the anchored element. Since bookmark links within the same page normally do not result in an HTTP request, one could argue that the default behavior of Turbolinks for links that have an href attribute beginning with # results in worse performance (due to the network request) and violates the principle of least surprise.

Is it possible to disable Turbolinks for all links that have an href attribute beginning with # without having to manually annotate them with data-turbolinks="false"?

I am currently using turbolinks (5.0.0.beta2) (turbolinks-source (5.0.0.beta4)) with Rails 4.2.6.

@packagethief

This comment has been minimized.

Show comment
Hide comment
@packagethief

packagethief Apr 15, 2016

Member

Since bookmark links within the same page normally do not result in an HTTP request, one could argue that the default behavior of Turbolinks for links that have an href attribute beginning with # results in worse performance (due to the network request) and violates the principle of least surprise.

I agree. We should ignore clicks on links that are anchors to the current page, and let them fall through to the browser.

Member

packagethief commented Apr 15, 2016

Since bookmark links within the same page normally do not result in an HTTP request, one could argue that the default behavior of Turbolinks for links that have an href attribute beginning with # results in worse performance (due to the network request) and violates the principle of least surprise.

I agree. We should ignore clicks on links that are anchors to the current page, and let them fall through to the browser.

@preetpalS

This comment has been minimized.

Show comment
Hide comment
@preetpalS

preetpalS Apr 17, 2016

Here is a workaround I tried to write to prevent Turbolinks from making unnecessary network calls (it is a compiled version of a gist I wrote in TypeScript). It works except for the scenario where if you're on a page like http://example.com, click on a link with an href that is only a hash like #bookmark, and then click on a link to a different page like http://example.com/other, you will no longer be able to go back in the browser (like from http://example.com/other to http://example.com#bookmark).

document.addEventListener("turbolinks:before-visit", function (event) {
    // console.info(`Network Call Avoidance Workaround (for Turbolinks) attempting to short-circuit network calls.`);
    var origin = window.location.href; // Current URL
    var destination = event.data.url; // Destination URL
    // Make sure both destination and origin urls end with "/" unless they contain an "#"
    if ((origin.match(/#/) === null) && origin[origin.length - 1] !== "/")
        origin = origin + '/';
    if ((destination.match(/#/) === null) && destination[destination.length - 1] !== "/")
        destination = destination + '/';
    // console.info(`Origin: ${origin}`);
    // console.info(`Destination: ${destination}`);
    // Prevent Turbolinks from doing anything when clicking on a link to the current page
    if (origin === destination) {
        // console.info("Turbolinks stopped since origin and destination URLs are identical.");
        event.preventDefault();
        if (origin.match(/#/) !== null) {
            // console.info(`Letting browser navigate to: ${destination}`);
            window.location.href = destination;
        }
        else {
            // Gives user feedback for clicking link to current page.
            // console.info(`Giving user fake feedback since link points to current page.`)
            var tpb = new Turbolinks.ProgressBar();
            tpb.setValue(0);
            tpb.show();
            tpb.setValue(0.2);
            setTimeout(function () { tpb.setValue(1); tpb.hide(); }, 100);
        }
        return;
    }
    var shorterLength = Math.min(origin.length, destination.length);
    // To handle the cases where both origin and destination URLs contain "#".
    // Only intended to prevent page reloads for anchor links within current page.
    if (((origin.match(/#/) !== null) && (destination.match(/#/) !== null)) &&
        (origin.indexOf("#") === destination.indexOf("#"))) {
        // console.info("Detected that both origin and destination have same index for #");
        shorterLength = Math.min(origin.indexOf("#"), destination.indexOf("#"));
    }
    // console.info(`shorterLength: ${shorterLength}`);
    // See if the first `shorterLength` characters of both the origin and destination URLS are the same.
    // If these characters are not the same, do nothing.
    if (origin.substring(0, shorterLength) !== destination.substring(0, shorterLength)) {
        // console.info(`Turbolinks not stopped since first ${shorterLength} characters of origin and destination URL are different.`);
        return;
    }
    // console.info(`The first ${shorterLength} chars of both origin and destination URLs are identical`);
    // If the destination URL contains an "#" immediately after the first `shorterLength` characters, let
    // the browser navigate to the URL. In this case the origin URL would be desination URL without the hash.
    if (destination.length > shorterLength && destination[shorterLength] === "#") {
        // console.info("Turbolinks stopped since destination URL is a bookmark on the current page.");
        event.preventDefault();
        // console.info(`Letting browser navigate to: ${destination}`);
        window.location.href = destination;
        return;
    }
    // If the origin URL contains an "#" immediately after the first `shorterLength` characters, do nothing.
    // In this case the destination URL would be the origin URL without the hash.
    if (origin.length > shorterLength && origin[shorterLength] === "#") {
        // console.info("Turbolinks stopped since trying to navigate to current page's URL without hash.");
        event.preventDefault();
        return;
    }
    // Note that either destination or origin is substring within the other (starting at index 0) if this function hasn't already returned.
    // console.info("Turbolinks not stopped since there is no reason to do so.");
});

preetpalS commented Apr 17, 2016

Here is a workaround I tried to write to prevent Turbolinks from making unnecessary network calls (it is a compiled version of a gist I wrote in TypeScript). It works except for the scenario where if you're on a page like http://example.com, click on a link with an href that is only a hash like #bookmark, and then click on a link to a different page like http://example.com/other, you will no longer be able to go back in the browser (like from http://example.com/other to http://example.com#bookmark).

document.addEventListener("turbolinks:before-visit", function (event) {
    // console.info(`Network Call Avoidance Workaround (for Turbolinks) attempting to short-circuit network calls.`);
    var origin = window.location.href; // Current URL
    var destination = event.data.url; // Destination URL
    // Make sure both destination and origin urls end with "/" unless they contain an "#"
    if ((origin.match(/#/) === null) && origin[origin.length - 1] !== "/")
        origin = origin + '/';
    if ((destination.match(/#/) === null) && destination[destination.length - 1] !== "/")
        destination = destination + '/';
    // console.info(`Origin: ${origin}`);
    // console.info(`Destination: ${destination}`);
    // Prevent Turbolinks from doing anything when clicking on a link to the current page
    if (origin === destination) {
        // console.info("Turbolinks stopped since origin and destination URLs are identical.");
        event.preventDefault();
        if (origin.match(/#/) !== null) {
            // console.info(`Letting browser navigate to: ${destination}`);
            window.location.href = destination;
        }
        else {
            // Gives user feedback for clicking link to current page.
            // console.info(`Giving user fake feedback since link points to current page.`)
            var tpb = new Turbolinks.ProgressBar();
            tpb.setValue(0);
            tpb.show();
            tpb.setValue(0.2);
            setTimeout(function () { tpb.setValue(1); tpb.hide(); }, 100);
        }
        return;
    }
    var shorterLength = Math.min(origin.length, destination.length);
    // To handle the cases where both origin and destination URLs contain "#".
    // Only intended to prevent page reloads for anchor links within current page.
    if (((origin.match(/#/) !== null) && (destination.match(/#/) !== null)) &&
        (origin.indexOf("#") === destination.indexOf("#"))) {
        // console.info("Detected that both origin and destination have same index for #");
        shorterLength = Math.min(origin.indexOf("#"), destination.indexOf("#"));
    }
    // console.info(`shorterLength: ${shorterLength}`);
    // See if the first `shorterLength` characters of both the origin and destination URLS are the same.
    // If these characters are not the same, do nothing.
    if (origin.substring(0, shorterLength) !== destination.substring(0, shorterLength)) {
        // console.info(`Turbolinks not stopped since first ${shorterLength} characters of origin and destination URL are different.`);
        return;
    }
    // console.info(`The first ${shorterLength} chars of both origin and destination URLs are identical`);
    // If the destination URL contains an "#" immediately after the first `shorterLength` characters, let
    // the browser navigate to the URL. In this case the origin URL would be desination URL without the hash.
    if (destination.length > shorterLength && destination[shorterLength] === "#") {
        // console.info("Turbolinks stopped since destination URL is a bookmark on the current page.");
        event.preventDefault();
        // console.info(`Letting browser navigate to: ${destination}`);
        window.location.href = destination;
        return;
    }
    // If the origin URL contains an "#" immediately after the first `shorterLength` characters, do nothing.
    // In this case the destination URL would be the origin URL without the hash.
    if (origin.length > shorterLength && origin[shorterLength] === "#") {
        // console.info("Turbolinks stopped since trying to navigate to current page's URL without hash.");
        event.preventDefault();
        return;
    }
    // Note that either destination or origin is substring within the other (starting at index 0) if this function hasn't already returned.
    // console.info("Turbolinks not stopped since there is no reason to do so.");
});
@preetpalS

This comment has been minimized.

Show comment
Hide comment
@preetpalS

preetpalS Apr 17, 2016

Here is another workaround (it is a compiled version of a gist (same gist mentioned in previous comment) I wrote in TypeScript). I will probably use this one in production. It just scrolls to bookmark links on the same page (it does not change the URL (or change browser history) but that's fine (for my purposes at least) since Turbolinks preserves scroll position while navigating through history anyways).

Warning: This workaround makes the assumption that your link will only contain a single # to mark the start of the hash (fragment (https://tools.ietf.org/html/rfc3986#section-3)) of your URL and that you are not using / as data in the query portion of your URL (https://tools.ietf.org/html/rfc3986#section-3.4).

/// <reference path="jquery/jquery.d.ts"/>
document.addEventListener("turbolinks:before-visit", function (event) {
    // console.info(`Network Call Avoidance Workaround (for Turbolinks) attempting to short-circuit network calls.`);
    var origin = window.location.href; // Current URL
    var destination = event.data.url; // Destination URL
    // console.info(`Original Origin: ${origin}`);
    // console.info(`Original Destination: ${destination}`);
    // Make sure both destination and origin urls do not end with "/"
    // If the contain a '#', ensure URLs do not have a '/' before the '#' (https://tools.ietf.org/html/rfc3986#section-3.4)
    if (origin.match(/#/) === null) {
        if (origin[origin.length - 1] === "/") {
            origin = origin.substring(0, origin.length - 1);
        }
    }
    else {
        var hashIndex = origin.indexOf('#');
        // console.info(hashIndex);
        if (hashIndex > 0 && origin[hashIndex - 1] === "/")
            origin = "" + origin.substring(0, (hashIndex - 1)) + origin.substring(hashIndex);
    }
    if (destination.match(/#/) === null) {
        if (destination[destination.length - 1] === "/") {
            destination = destination.substring(0, destination.length - 1);
        }
    }
    else {
        var hashIndex = destination.indexOf('#');
        // console.info(hashIndex);
        if (hashIndex > 0 && destination[hashIndex - 1] === "/")
            destination = "" + destination.substring(0, (hashIndex - 1)) + destination.substring(hashIndex);
    }
    // console.info(`Modified Origin: ${origin}`);
    // console.info(`Modified Destination: ${destination}`);
    // Do not prevent Turbolinks from following links to the same URL (can affect forms)
    if (origin === destination)
        return;
    var shorterLength = Math.min(origin.length, destination.length);
    // To handle the cases where both origin and destination URLs contain "#".
    // Only intended to prevent page reloads for anchor links within current page.
    if (((origin.match(/#/) !== null) && (destination.match(/#/) !== null)) &&
        (origin.indexOf("#") === destination.indexOf("#"))) {
        // console.info("Detected that both origin and destination have same index for #");
        shorterLength = Math.min(origin.indexOf("#"), destination.indexOf("#"));
    }
    // console.info(`shorterLength: ${shorterLength}`);
    // See if the first `shorterLength` characters of both the origin and destination URLS are the same.
    // If these characters are not the same, do nothing.
    if (origin.substring(0, shorterLength) !== destination.substring(0, shorterLength)) {
        // console.info(`Turbolinks not stopped since first ${shorterLength} characters of origin and destination URL are different.`);
        return;
    }
    // console.info(`The first ${shorterLength} chars of both origin and destination URLs are identical`);
    // If the destination URL contains an "#" immediately after the first `shorterLength` characters, let
    // the browser navigate to the URL. In this case the origin URL would be desination URL without the hash.
    if (destination.length > shorterLength && destination[shorterLength] === "#") {
        // console.info("Turbolinks stopped since destination URL is a bookmark on the current page.");
        event.preventDefault();
        // This will mess up browser history and prevent you from going back after moving forward
        // window.location.href = destination;
        var urlHashMinusHash = destination.substring(destination.indexOf('#')).substring(1);
        // console.info(`Letting browser scroll to: ${urlHashMinusHash}`);
        var elem = document.getElementById(urlHashMinusHash);
        if (elem != null)
            elem.scrollIntoView();
        return;
    }
    // If the origin URL contains an "#" immediately after the first `shorterLength` characters, do nothing.
    // In this case the destination URL would be the origin URL without the hash.
    if (origin.length > shorterLength && origin[shorterLength] === "#") {
        // console.info("Turbolinks stopped since trying to navigate to current page's URL without hash.");
        event.preventDefault();
        $("html, body").animate({ scrollTop: 0 }, "slow");
        return;
    }
    // Note that either destination or origin is substring within the other (starting at index 0) if this function hasn't already returned.
    // console.info("Turbolinks not stopped since there is no reason to do so.");
});

preetpalS commented Apr 17, 2016

Here is another workaround (it is a compiled version of a gist (same gist mentioned in previous comment) I wrote in TypeScript). I will probably use this one in production. It just scrolls to bookmark links on the same page (it does not change the URL (or change browser history) but that's fine (for my purposes at least) since Turbolinks preserves scroll position while navigating through history anyways).

Warning: This workaround makes the assumption that your link will only contain a single # to mark the start of the hash (fragment (https://tools.ietf.org/html/rfc3986#section-3)) of your URL and that you are not using / as data in the query portion of your URL (https://tools.ietf.org/html/rfc3986#section-3.4).

/// <reference path="jquery/jquery.d.ts"/>
document.addEventListener("turbolinks:before-visit", function (event) {
    // console.info(`Network Call Avoidance Workaround (for Turbolinks) attempting to short-circuit network calls.`);
    var origin = window.location.href; // Current URL
    var destination = event.data.url; // Destination URL
    // console.info(`Original Origin: ${origin}`);
    // console.info(`Original Destination: ${destination}`);
    // Make sure both destination and origin urls do not end with "/"
    // If the contain a '#', ensure URLs do not have a '/' before the '#' (https://tools.ietf.org/html/rfc3986#section-3.4)
    if (origin.match(/#/) === null) {
        if (origin[origin.length - 1] === "/") {
            origin = origin.substring(0, origin.length - 1);
        }
    }
    else {
        var hashIndex = origin.indexOf('#');
        // console.info(hashIndex);
        if (hashIndex > 0 && origin[hashIndex - 1] === "/")
            origin = "" + origin.substring(0, (hashIndex - 1)) + origin.substring(hashIndex);
    }
    if (destination.match(/#/) === null) {
        if (destination[destination.length - 1] === "/") {
            destination = destination.substring(0, destination.length - 1);
        }
    }
    else {
        var hashIndex = destination.indexOf('#');
        // console.info(hashIndex);
        if (hashIndex > 0 && destination[hashIndex - 1] === "/")
            destination = "" + destination.substring(0, (hashIndex - 1)) + destination.substring(hashIndex);
    }
    // console.info(`Modified Origin: ${origin}`);
    // console.info(`Modified Destination: ${destination}`);
    // Do not prevent Turbolinks from following links to the same URL (can affect forms)
    if (origin === destination)
        return;
    var shorterLength = Math.min(origin.length, destination.length);
    // To handle the cases where both origin and destination URLs contain "#".
    // Only intended to prevent page reloads for anchor links within current page.
    if (((origin.match(/#/) !== null) && (destination.match(/#/) !== null)) &&
        (origin.indexOf("#") === destination.indexOf("#"))) {
        // console.info("Detected that both origin and destination have same index for #");
        shorterLength = Math.min(origin.indexOf("#"), destination.indexOf("#"));
    }
    // console.info(`shorterLength: ${shorterLength}`);
    // See if the first `shorterLength` characters of both the origin and destination URLS are the same.
    // If these characters are not the same, do nothing.
    if (origin.substring(0, shorterLength) !== destination.substring(0, shorterLength)) {
        // console.info(`Turbolinks not stopped since first ${shorterLength} characters of origin and destination URL are different.`);
        return;
    }
    // console.info(`The first ${shorterLength} chars of both origin and destination URLs are identical`);
    // If the destination URL contains an "#" immediately after the first `shorterLength` characters, let
    // the browser navigate to the URL. In this case the origin URL would be desination URL without the hash.
    if (destination.length > shorterLength && destination[shorterLength] === "#") {
        // console.info("Turbolinks stopped since destination URL is a bookmark on the current page.");
        event.preventDefault();
        // This will mess up browser history and prevent you from going back after moving forward
        // window.location.href = destination;
        var urlHashMinusHash = destination.substring(destination.indexOf('#')).substring(1);
        // console.info(`Letting browser scroll to: ${urlHashMinusHash}`);
        var elem = document.getElementById(urlHashMinusHash);
        if (elem != null)
            elem.scrollIntoView();
        return;
    }
    // If the origin URL contains an "#" immediately after the first `shorterLength` characters, do nothing.
    // In this case the destination URL would be the origin URL without the hash.
    if (origin.length > shorterLength && origin[shorterLength] === "#") {
        // console.info("Turbolinks stopped since trying to navigate to current page's URL without hash.");
        event.preventDefault();
        $("html, body").animate({ scrollTop: 0 }, "slow");
        return;
    }
    // Note that either destination or origin is substring within the other (starting at index 0) if this function hasn't already returned.
    // console.info("Turbolinks not stopped since there is no reason to do so.");
});
@glennfu

This comment has been minimized.

Show comment
Hide comment
@glennfu

glennfu Apr 19, 2016

@preetpalS I tried your 2nd code snippet on my app, and while it did indeed prevent Turbolinks from intercepting hash changes which was awesome, it had the side effect of blocking Turbolinks.visit("http://myapp.dev/current/page", {"action":"replace"})

I'm doing a remote form submission which reloads the same page after submit, and after adding this block of code, I still see the progress bar animate, but the Turbolinks visitProposedToLocationWithAction never gets triggered and the page doesn't refresh as it should.

A fix might be to change line 50 if (origin === destination) { to also take into account whether or not the visit's action was 'advance' or not, but this information isn't currently passed to the "turbolinks:before-visit" event, so I'm not sure what should be done here.

glennfu commented Apr 19, 2016

@preetpalS I tried your 2nd code snippet on my app, and while it did indeed prevent Turbolinks from intercepting hash changes which was awesome, it had the side effect of blocking Turbolinks.visit("http://myapp.dev/current/page", {"action":"replace"})

I'm doing a remote form submission which reloads the same page after submit, and after adding this block of code, I still see the progress bar animate, but the Turbolinks visitProposedToLocationWithAction never gets triggered and the page doesn't refresh as it should.

A fix might be to change line 50 if (origin === destination) { to also take into account whether or not the visit's action was 'advance' or not, but this information isn't currently passed to the "turbolinks:before-visit" event, so I'm not sure what should be done here.

@preetpalS

This comment has been minimized.

Show comment
Hide comment
@preetpalS

preetpalS Apr 19, 2016

@glennfu Try out the updated version of the 2nd code snippet.

Another completely different workaround that might work better would be to add an event listener on the turbolinks:load event that searches the body of the page for all HTML link elements (ignoring links already annotated with data-turbolinks) which are just links to bookmarks on the current page (maybe just filter based on those which have an href attribute beginning with # or possibly filter based on a more involved comparison between the current URL and where those links point to) and annotates the found link elements with data-turbolinks="false".

preetpalS commented Apr 19, 2016

@glennfu Try out the updated version of the 2nd code snippet.

Another completely different workaround that might work better would be to add an event listener on the turbolinks:load event that searches the body of the page for all HTML link elements (ignoring links already annotated with data-turbolinks) which are just links to bookmarks on the current page (maybe just filter based on those which have an href attribute beginning with # or possibly filter based on a more involved comparison between the current URL and where those links point to) and annotates the found link elements with data-turbolinks="false".

@glennfu

This comment has been minimized.

Show comment
Hide comment
@glennfu

glennfu May 11, 2016

I just realized a 2nd use case for having the action passed along in this event: If I'm on url "/foo/bar" and I want to do Turbolinks.visit("/foo/bar#panel-this", {"action":"replace"}) I would expect the page to reload, and land me on the url "/foo/bar#panel-this".

I can't think of a way to update this code to satisfy that without having access to the action

glennfu commented May 11, 2016

I just realized a 2nd use case for having the action passed along in this event: If I'm on url "/foo/bar" and I want to do Turbolinks.visit("/foo/bar#panel-this", {"action":"replace"}) I would expect the page to reload, and land me on the url "/foo/bar#panel-this".

I can't think of a way to update this code to satisfy that without having access to the action

@sstephenson sstephenson changed the title from Can Turbolinks ignore all bookmark links that do not require making a HTTP request to Turbolinks should follow same-page anchor links without reloading the page Jun 20, 2016

@wshostak

This comment has been minimized.

Show comment
Hide comment
@wshostak

wshostak Jul 5, 2016

The quick solution I have come up with (not fully tested);

Turbolinks.Controller.prototype.nodeIsVisitableOld = Turbolinks.Controller.prototype.nodeIsVisitable;

Turbolinks.Controller.prototype.nodeIsVisitable = function (e) {

  return !document.querySelector(elem.getAttribute('href')) && Turbolinks.Controller.prototype.nodeIsVisitableOld(e)
};

I figure if the anchor exist then the node is also not visitable.
Best to load this code right after loading turbolinks.

wshostak commented Jul 5, 2016

The quick solution I have come up with (not fully tested);

Turbolinks.Controller.prototype.nodeIsVisitableOld = Turbolinks.Controller.prototype.nodeIsVisitable;

Turbolinks.Controller.prototype.nodeIsVisitable = function (e) {

  return !document.querySelector(elem.getAttribute('href')) && Turbolinks.Controller.prototype.nodeIsVisitableOld(e)
};

I figure if the anchor exist then the node is also not visitable.
Best to load this code right after loading turbolinks.

@vickodin

This comment has been minimized.

Show comment
Hide comment
@vickodin

vickodin Jul 14, 2016

Waiting for news 👍

Waiting for news 👍

@wshostak

This comment has been minimized.

Show comment
Hide comment
@wshostak

wshostak Jul 14, 2016

Thanks for reminding me to post the changes I have made.

Turbolinks.Controller.prototype.nodeIsVisitableOld = Turbolinks.Controller.prototype.nodeIsVisitable;

Turbolinks.Controller.prototype.nodeIsVisitable = function (elem) {

  var href = elem.getAttribute('href') || '',
      anchor = href[0] === "#"? anchor = document.querySelector(href): false;

  return !anchor && Turbolinks.Controller.prototype.nodeIsVisitableOld(elem);
};

Thanks for reminding me to post the changes I have made.

Turbolinks.Controller.prototype.nodeIsVisitableOld = Turbolinks.Controller.prototype.nodeIsVisitable;

Turbolinks.Controller.prototype.nodeIsVisitable = function (elem) {

  var href = elem.getAttribute('href') || '',
      anchor = href[0] === "#"? anchor = document.querySelector(href): false;

  return !anchor && Turbolinks.Controller.prototype.nodeIsVisitableOld(elem);
};
@vtamara

This comment has been minimized.

Show comment
Hide comment
@vtamara

vtamara Jul 21, 2016

The only workaround that initially worked for me was to change in Gemfile:

 gem "turbolinks", '2.5.3' 

11.Jul.2017: After digging more, I found that the problem I was having was not with links to anchors but a feature in turbolinks-rails that shows in navigator what is sent by redirect_to even if using AJAX with POST. I could fix and now I'm using the most recent turbolinks, thank you.
More about the problem I had and the solution at: https://github.com/vtamara/turbolinks_prob50

vtamara commented Jul 21, 2016

The only workaround that initially worked for me was to change in Gemfile:

 gem "turbolinks", '2.5.3' 

11.Jul.2017: After digging more, I found that the problem I was having was not with links to anchors but a feature in turbolinks-rails that shows in navigator what is sent by redirect_to even if using AJAX with POST. I could fix and now I'm using the most recent turbolinks, thank you.
More about the problem I had and the solution at: https://github.com/vtamara/turbolinks_prob50

@jeremylynch

This comment has been minimized.

Show comment
Hide comment
@jeremylynch

jeremylynch Aug 3, 2016

@wshostak your path has worked for me.

Any progress in patching this bug in the turbolinks master?

Update: @wshostak's solution works, except after clicking an anchor, the back event does not function correctly.

jeremylynch commented Aug 3, 2016

@wshostak your path has worked for me.

Any progress in patching this bug in the turbolinks master?

Update: @wshostak's solution works, except after clicking an anchor, the back event does not function correctly.

@jeremylynch

This comment has been minimized.

Show comment
Hide comment
@jeremylynch

jeremylynch Aug 29, 2016

@sstephenson, @packagethief, @wshostak & @preetpalS what can I do to help move this forward? Unfortunately I don't have the JS expertise to solve this. This is quite a critical flaw of turbolinks.

jeremylynch commented Aug 29, 2016

@sstephenson, @packagethief, @wshostak & @preetpalS what can I do to help move this forward? Unfortunately I don't have the JS expertise to solve this. This is quite a critical flaw of turbolinks.

@domchristie

This comment has been minimized.

Show comment
Hide comment
@domchristie

domchristie Sep 6, 2016

Contributor

Another fix which does not involve modifying Turbolinks internal methods (and I think does not break "Back" behaviour):

$(document).on('turbolinks:click', function (event) {
  if (event.target.getAttribute('href').charAt(0) === '#') {
    return event.preventDefault()
  }
})
Contributor

domchristie commented Sep 6, 2016

Another fix which does not involve modifying Turbolinks internal methods (and I think does not break "Back" behaviour):

$(document).on('turbolinks:click', function (event) {
  if (event.target.getAttribute('href').charAt(0) === '#') {
    return event.preventDefault()
  }
})
@jeremylynch

This comment has been minimized.

Show comment
Hide comment
@jeremylynch

jeremylynch Sep 6, 2016

@domchristie, I have tested your solution and it still breaks back behaviour for me.

@domchristie, I have tested your solution and it still breaks back behaviour for me.

@domchristie

This comment has been minimized.

Show comment
Hide comment
@domchristie

domchristie Sep 7, 2016

Contributor

@mrjlynch ah: it does not break "Back" behaviour … sometimes! I seem to get different behaviours, although I am not able to reproduce them consistently. Navigating "Back" either takes me back to the top of the page after a Turbolinks page load (which seems acceptable), or it updates the address bar to the previous URL but does not update the scroll position.

Contributor

domchristie commented Sep 7, 2016

@mrjlynch ah: it does not break "Back" behaviour … sometimes! I seem to get different behaviours, although I am not able to reproduce them consistently. Navigating "Back" either takes me back to the top of the page after a Turbolinks page load (which seems acceptable), or it updates the address bar to the previous URL but does not update the scroll position.

@wjdp

This comment has been minimized.

Show comment
Hide comment
@wjdp

wjdp Sep 8, 2016

@domchristie @mrjlynch I've just come across this issue, currently working around using data-turbolinks="false" on the links. I'm getting broken Back behaviour with this method. Seems as if unless a request goes through TL fully it doesn't get a network request on a browser back action.

wjdp commented Sep 8, 2016

@domchristie @mrjlynch I've just come across this issue, currently working around using data-turbolinks="false" on the links. I'm getting broken Back behaviour with this method. Seems as if unless a request goes through TL fully it doesn't get a network request on a browser back action.

@jeremylynch

This comment has been minimized.

Show comment
Hide comment
@jeremylynch

jeremylynch Sep 9, 2016

@domchristie's solution (below) is the best solution I have found yet.

$(document).on('turbolinks:click', function (event) {
  if (event.target.getAttribute('href').charAt(0) === '#') {
    return event.preventDefault()
  }
})

Why?

  1. It is the shortest solution.
  2. When anchor links have html child elements (see example below), clicking the child does not trigger reload (which is a fault of @wshostak solution).
<a href="#anchor">
    <i class="icon"></i>
</a>

The problem to fix:

  1. Clicking the back button

jeremylynch commented Sep 9, 2016

@domchristie's solution (below) is the best solution I have found yet.

$(document).on('turbolinks:click', function (event) {
  if (event.target.getAttribute('href').charAt(0) === '#') {
    return event.preventDefault()
  }
})

Why?

  1. It is the shortest solution.
  2. When anchor links have html child elements (see example below), clicking the child does not trigger reload (which is a fault of @wshostak solution).
<a href="#anchor">
    <i class="icon"></i>
</a>

The problem to fix:

  1. Clicking the back button
@glennfu

This comment has been minimized.

Show comment
Hide comment
@glennfu

glennfu Sep 9, 2016

Can anyone publish a small demo project demonstrating the problem? Right now in my app, with the latest version of Turbolinks, hash changes are ignored, which is actually correct and preferable for me. I don't actually want the back button to affect the hash in my project so I don't have a valid use case of my own to test against. However if someone can put up something that clearly shows the problem I'd be interested in tinkering with a solution that actually leverages the Turbolinks cache and the restorationData that stores the scroll position. My theory is that if in the "restore" process I can convince Turbolinks to skip the re-render if the only part of the url changing is the anchor, then maybe we can have a better solution. We'd also want to make sure that the default behavior of ignoring hash changes can remain as an option as well.

I think a good test case would be to have a page that goes from /url1 -> /url2 -> /url2#anchor1 -> /url2#anchor2 -> /url1 and make sure that the history and navigation all play nicely.

glennfu commented Sep 9, 2016

Can anyone publish a small demo project demonstrating the problem? Right now in my app, with the latest version of Turbolinks, hash changes are ignored, which is actually correct and preferable for me. I don't actually want the back button to affect the hash in my project so I don't have a valid use case of my own to test against. However if someone can put up something that clearly shows the problem I'd be interested in tinkering with a solution that actually leverages the Turbolinks cache and the restorationData that stores the scroll position. My theory is that if in the "restore" process I can convince Turbolinks to skip the re-render if the only part of the url changing is the anchor, then maybe we can have a better solution. We'd also want to make sure that the default behavior of ignoring hash changes can remain as an option as well.

I think a good test case would be to have a page that goes from /url1 -> /url2 -> /url2#anchor1 -> /url2#anchor2 -> /url1 and make sure that the history and navigation all play nicely.

@domchristie

This comment has been minimized.

Show comment
Hide comment
@domchristie

domchristie Sep 11, 2016

Contributor

@glennfu Here is the test case project as requested. As you can see by monitoring the XHRs, clicking on same-page anchor links causes requests to be made. This is probably acceptable in many cases, but given that the content should exist on the page, there shouldn't be a need to make extraneous requests.

To further demonstrate the problems, here is another case: a blog with lazily-loaded comments. Here is what happens:

  1. User visits a posts#show page
  2. Comments for that post are loaded via XHR
  3. User clicks to view Comment # 1 (an anchor on the same page)
  4. Turbolinks fetches the posts#show page again and replaces the <body> (without the comments)
  5. Turbolinks then tries to scroll to Comment # 1, which does not exist anymore, and so the fragment identifier is ignored :(

Clicking on a same-page anchor link with data-turbolinks=false prevents the reload and scrolls to the anchor as expected, however pressing Back does not always return the app to the previous state.

Contributor

domchristie commented Sep 11, 2016

@glennfu Here is the test case project as requested. As you can see by monitoring the XHRs, clicking on same-page anchor links causes requests to be made. This is probably acceptable in many cases, but given that the content should exist on the page, there shouldn't be a need to make extraneous requests.

To further demonstrate the problems, here is another case: a blog with lazily-loaded comments. Here is what happens:

  1. User visits a posts#show page
  2. Comments for that post are loaded via XHR
  3. User clicks to view Comment # 1 (an anchor on the same page)
  4. Turbolinks fetches the posts#show page again and replaces the <body> (without the comments)
  5. Turbolinks then tries to scroll to Comment # 1, which does not exist anymore, and so the fragment identifier is ignored :(

Clicking on a same-page anchor link with data-turbolinks=false prevents the reload and scrolls to the anchor as expected, however pressing Back does not always return the app to the previous state.

@aguynamedben

This comment has been minimized.

Show comment
Hide comment
@aguynamedben

aguynamedben Oct 24, 2016

Thanks for your work on Turbolinks 5. So far it's VERY awesome.

An important business side effect of this behavior is that analytics for pages with same-page anchor links can be overstated due to the 'turbolinks:load' event firing after clicks on same-page anchor links. This is common with a Table of Contents or other intra-page linking. For example, in order to implement Google Analytics (analytics.js) on a Turbolinks app, we're doing:

$(document).on('turbolinks:load', function(event) {
  ga('set', 'page', window.location.pathname);
  ga('send', 'pageview');
});

Because Turbolinks is intercepting the clicks on same-page anchor links (and doing more work than it should), the 'turbolinks:load' event is fired too many times and page views are being overstated.

With my Table of Contents use case, 'turbolinks:load' events fire for:

/guide
/guide#section3 (user clicks in TOC, same page)
/guide#section4 (user clicks in TOC, same page)

and 1 page view becomes 3 page views.

aguynamedben commented Oct 24, 2016

Thanks for your work on Turbolinks 5. So far it's VERY awesome.

An important business side effect of this behavior is that analytics for pages with same-page anchor links can be overstated due to the 'turbolinks:load' event firing after clicks on same-page anchor links. This is common with a Table of Contents or other intra-page linking. For example, in order to implement Google Analytics (analytics.js) on a Turbolinks app, we're doing:

$(document).on('turbolinks:load', function(event) {
  ga('set', 'page', window.location.pathname);
  ga('send', 'pageview');
});

Because Turbolinks is intercepting the clicks on same-page anchor links (and doing more work than it should), the 'turbolinks:load' event is fired too many times and page views are being overstated.

With my Table of Contents use case, 'turbolinks:load' events fire for:

/guide
/guide#section3 (user clicks in TOC, same page)
/guide#section4 (user clicks in TOC, same page)

and 1 page view becomes 3 page views.

@brandoncc

This comment has been minimized.

Show comment
Hide comment
@brandoncc

brandoncc Jul 11, 2017

I added this to turbolinks_compatibility.coffee:

# Added to fix turbolinks anchor issue
# https://github.com/turbolinks/turbolinks/issues/75#issuecomment-244915109
linkTargetsAnchorOnSamePage = (link) ->
  href = link.getAttribute('href')

  return true if href.charAt(0) == '#'

  if href.match(new RegExp('^' + window.location.toString().replace(/#.*/, '') + '#'))
    return true
  else if href.match(new RegExp('^' + window.location.pathname + '#'))
    return true

  return false
  
$(document).on 'turbolinks:click', (event) ->
  if linkTargetsAnchorOnSamePage(event.target)
    return event.preventDefault()

$(document).on 'turbolinks:load', (event) ->
  if window.location.hash
    $element = $('a[name="' + window.location.hash.substring(1) + '"]')
    $('html, body').scrollTop($element.offset().top)

And it seems to be working well for me. There may be some back button weirdness but it seemed to work acceptably.

I added this to turbolinks_compatibility.coffee:

# Added to fix turbolinks anchor issue
# https://github.com/turbolinks/turbolinks/issues/75#issuecomment-244915109
linkTargetsAnchorOnSamePage = (link) ->
  href = link.getAttribute('href')

  return true if href.charAt(0) == '#'

  if href.match(new RegExp('^' + window.location.toString().replace(/#.*/, '') + '#'))
    return true
  else if href.match(new RegExp('^' + window.location.pathname + '#'))
    return true

  return false
  
$(document).on 'turbolinks:click', (event) ->
  if linkTargetsAnchorOnSamePage(event.target)
    return event.preventDefault()

$(document).on 'turbolinks:load', (event) ->
  if window.location.hash
    $element = $('a[name="' + window.location.hash.substring(1) + '"]')
    $('html, body').scrollTop($element.offset().top)

And it seems to be working well for me. There may be some back button weirdness but it seemed to work acceptably.

@obromios

This comment has been minimized.

Show comment
Hide comment
@obromios

obromios Jul 12, 2017

@brandoncc, your comment worked for me.

@brandoncc, your comment worked for me.

taw added a commit to taw/magic-search-engine that referenced this issue Jul 24, 2017

peichman-umd added a commit to peichman-umd/archelon that referenced this issue Aug 2, 2017

LIBHYDRA-103. Move image viewer above metadata.
Includes a "skip link" to the metadata. This link required a data-turbolinks="false" attribute to stop TurboLinks from doing another GET request for the in-page link. See also turbolinks/turbolinks#75.

https://issues.umd.edu/browse/LIBHYDRA-103
@dennisreimann

This comment has been minimized.

Show comment
Hide comment
@dennisreimann

dennisreimann Aug 24, 2017

The problem here IMHO is that Turbolinks assumes there is something to do onPopState invokation for same page anchors.

The onpopstate docs mention two cases for triggering the event:

The popstate event is only triggered by performing a browser action, such as clicking on the back button (or calling history.back() in JavaScript), when navigating between two history entries for the same document.

The first case (navigation via the browser back/forward buttons) is handled in the History module shouldHandlePopState method, the second one (navigation via anchors in the same document) isn't.

I already tried passing the onPopState event to shouldHandlePopState and comparing the current and target location. Unfortunately this is too late, as both locations are always equal, which makes sense as the popstate event gets triggered after the URL has changed.

dennisreimann commented Aug 24, 2017

The problem here IMHO is that Turbolinks assumes there is something to do onPopState invokation for same page anchors.

The onpopstate docs mention two cases for triggering the event:

The popstate event is only triggered by performing a browser action, such as clicking on the back button (or calling history.back() in JavaScript), when navigating between two history entries for the same document.

The first case (navigation via the browser back/forward buttons) is handled in the History module shouldHandlePopState method, the second one (navigation via anchors in the same document) isn't.

I already tried passing the onPopState event to shouldHandlePopState and comparing the current and target location. Unfortunately this is too late, as both locations are always equal, which makes sense as the popstate event gets triggered after the URL has changed.

@domchristie

This comment has been minimized.

Show comment
Hide comment
@domchristie

domchristie Sep 7, 2017

Contributor

I have created a PR to fix this, and a basic app to demonstrate the fix:

https://agile-reef-88023-fix.herokuapp.com/posts/1

It's a blogging app with comments loaded after the post loads (as described here). The "Jump to Comment 1" should now work as expected. It bypasses any rendering and avoids triggering turbolinks:load.

If you find any issues please note them on that PR. Thanks 😊

Contributor

domchristie commented Sep 7, 2017

I have created a PR to fix this, and a basic app to demonstrate the fix:

https://agile-reef-88023-fix.herokuapp.com/posts/1

It's a blogging app with comments loaded after the post loads (as described here). The "Jump to Comment 1" should now work as expected. It bypasses any rendering and avoids triggering turbolinks:load.

If you find any issues please note them on that PR. Thanks 😊

@brandoncc

This comment has been minimized.

Show comment
Hide comment

Nicely done @domchristie!

@sergeych

This comment has been minimized.

Show comment
Hide comment
@sergeych

sergeych Nov 6, 2017

So will we ever have this bug fixed? it is really annoying

sergeych commented Nov 6, 2017

So will we ever have this bug fixed? it is really annoying

@jujudellago

This comment has been minimized.

Show comment
Hide comment
@jujudellago

jujudellago Feb 14, 2018

I still got the bug now.

Adding the turbolinks_compatibility.coffee mentionned above did solve the problem.

I still got the bug now.

Adding the turbolinks_compatibility.coffee mentionned above did solve the problem.

@soundasleep

This comment has been minimized.

Show comment
Hide comment
@soundasleep

soundasleep Feb 22, 2018

This seems to have been the cause of a very weird Javascript error in my project, occurring within (uglified) t.Controller</n.prototype.nodeIsVisitable.

TypeError: 'closest' called on an object that does not implement interface Element.

It was because I had added a .on('click', ...) to an element <a href="#">, but the click function did not preventDefault, so Turbolinks was trying to do something bizarre with reloading the page.

This seems to have been the cause of a very weird Javascript error in my project, occurring within (uglified) t.Controller</n.prototype.nodeIsVisitable.

TypeError: 'closest' called on an object that does not implement interface Element.

It was because I had added a .on('click', ...) to an element <a href="#">, but the click function did not preventDefault, so Turbolinks was trying to do something bizarre with reloading the page.

georgemillo added a commit to dradis/dradis-ce that referenced this issue Jul 19, 2018

add data-turbolinks false to comment anchor link
Without this attribute, clicking the link triggers a full turbolinks
page reload rather than just jumping to the correct spot on the page
like you'd expect with a normal HTML same-page anchor tag.

This is a known issue with turbolinks and it doesn't look like
it's going to be fixed soon (the issue linked below is still open
afte two years); in the meantime the solution is to add this data
attribute.

See:

  turbolinks/turbolinks#75
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment