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

Consider switching to javascript:void(0) for hrefs #916

Closed
jgable opened this issue Sep 27, 2013 · 20 comments
Closed

Consider switching to javascript:void(0) for hrefs #916

jgable opened this issue Sep 27, 2013 · 20 comments
Assignees
Labels
affects:admin Anything relating to Ghost Admin

Comments

@jgable
Copy link
Contributor

jgable commented Sep 27, 2013

I know this can be kind of contentious / personal taste issue, but the #'s in our urls can potentially cause us problems down the road with backbones routing on browsers like IE9 that don't support pushState.

We should consider switching to javascript:void(0) to avoid changing the url when clicking about. Or, we could start using button's where semantically they would be more correct.

@gotdibbs
Copy link
Contributor

+1 to this one, didn't notice the #'s before but I've been bit by this before.

@cgiffard
Copy link
Contributor

I thought we weren't planning to support IE < 9?

@gotdibbs
Copy link
Contributor

@cgiffard Even IE9 doesn't support pushState 😢

@nicoburns
Copy link
Contributor

Is this issue mitigated by jashkenas/backbone#2766?

@jgable
Copy link
Contributor Author

jgable commented Oct 1, 2013

Whether or not it is, I would still vote against them because they force the # appended on the url which I find offends my delicate southern sensibilities.

@nicoburns
Copy link
Contributor

How about a JavaScript solution? I'm going to propose a function like this for intercepting URLs in order to make pushstate work with ordinary links.

hijackLinks: function(ev) {
    var $target = $(ev.currentTarget);

    // Get the absolute anchor href.
    var href = { prop: $target.prop("href"), attr: $target.attr("href") };
    // Get the absolute root.
    var root = location.protocol + "//" + location.host + urls.root;

    // Ensure the root is part of the anchor href, meaning it's relative.
    if (href.prop.slice(0, root.length) === root) {
        // Stop the default event to ensure the link will not cause a page
        // refresh.
        ev.preventDefault();

        // `Backbone.history.navigate` is sufficient for all Routers and will
        // trigger the correct events. The Router's internal `navigate` method
        // calls this anyways.  The fragment is sliced from the root.
        Backbone.history.navigate(href.attr, true);
    }
}

$(document.body).on("click", "a[href]:not([data-bypass])", hijackLinks);

We could easily add a case to ignore href === "#".

@JohnONolan
Copy link
Member

Couple of thoughts:

First - <button> usage may be semantic - but it's a huge cross browser pain in the ass. Primarily for the reason that (and I'm not going to sugar coat it) Firefox is a ridiculous pile of absolute shit which embeds !important declarations in its user agent stylesheet.

There is a 7+ year old ticket about this on bugzilla - https://bugzilla.mozilla.org/show_bug.cgi?id=349259 - indicating they have zero intention of changing it. The long and short of the issue is that it's impossible to properly set the height of a <button> with consistent padding. So... there's that.

Second - and I'm not sure how relevant this is to this particular issue - but without an href existing, <a> has no hover state applied to it by the browser.

@ErisDS
Copy link
Member

ErisDS commented Nov 28, 2013

I think what @jgable is getting at is using href="javascript:void(0)" rather than href="#"

@jgable
Copy link
Contributor Author

jgable commented Nov 28, 2013

Yeah, the button thing sounds like a pain, the javascript:void(0) can look a little ugly though.

@bastilian
Copy link
Contributor

I would suggest setting the href's to a useful path and remove it where it is not applicable.

So, let's get rid of the #'s!

...and yes, @JohnONolan the "Don't touch my UI elements"-attitude in Firefox is really annoying, I remember when they did it hardcore in Safari too...

@nicoburns
Copy link
Contributor

@bastilian brings up a good point: if we are going to support Ghost without JavaScript (as in #1205), then links are going to need to point to a real server address (which can then be caught and redirected to Backbone as necessary when JavaScript is enabled).

@GarrettS
Copy link

GarrettS commented Dec 5, 2013

Don't use javascript:void(0);

I have <a href="javascript:somefunction()"> what ... ?

Whatever the rest of your question, this is generally a very bad idea.
The javascript: pseudo protocol was designed to replace the
current document with the value that is returned from the expression.
For example:

  <a href="javascript:'&lt;h1&gt;' + document.lastModified + '&lt;/h1&gt;'">lastModified</a>

will result in replacing the current document with the value returned from document.lastModified, wrapped in an <h1> element.

When the expression used evaluates to an undefined value
(as some function calls do), the contents of the current page are not
replaced. Regardless, some browsers (notably IE6) interpret this as
navigation and will enter into a 'navigation' state where GIF
animations and plugins (such as movies) will stop and navigational
features such as META refresh, assignment to location.href, and image
swaps fail.

It is also possible for IE to be configured such that it supports
javascript but not the javascript: protocol. This results
in the user seeing a protocol error for javascript: URIs.

The javascript: pseudo protocol creates accessibility and
usability problems. It provides no fallback for when the script is not
supported.

Instead, use
<a href="something.html" onclick="somefunction();return false">
where something.html is a meaningful alternative. Alternatively,
attach the click callback using an event registry.

http://jsfaq.org/#javascriptURI

@karlmikko
Copy link
Contributor

In the interest of saving a few bites - if it is decided to use the javascript:void() I have found that javascript:// is shorted and works everywhere.

@ErisDS ErisDS modified the milestones: Ember.js, Multi-user Apr 15, 2014
@ErisDS
Copy link
Member

ErisDS commented Apr 15, 2014

Am I right in thinking Ember solves this for us and that this can be closed purely as a fact of switching to Ember?

@jgable
Copy link
Contributor Author

jgable commented Apr 15, 2014

It looks like the <a> tag no longer has an href in Ember, at least on the settings/general work that I've been doing. Which really means that we have to either put it in there and still make this decision, or switch it to a button.

@hswolff
Copy link
Contributor

hswolff commented Apr 15, 2014

I've noticed that in Ember if you have href="#" it causes the Ember router to break as the # is added to the url and routing seems to stop.

So at the very least we probably shouldn't use href="#".

@ErisDS
Copy link
Member

ErisDS commented Apr 17, 2014

Sounds like we need a global solution / convention for this then as part of the Ember project

@ErisDS ErisDS added the client label Apr 17, 2014
@hswolff
Copy link
Contributor

hswolff commented Apr 17, 2014

Any anchor elements that navigate to a route should naturally use the Ember {{link-to}} helper which generates a real anchor element with its href attribute filled.

All other elements that just have JavaScript actions attached shouldn't be anchor elements IMO. It's not what anchor elements are intended to be used for.

@ErisDS
Copy link
Member

ErisDS commented Jun 1, 2014

So - use {{#link-to}} or a button is the solution, I've assigned this to @PaulAdamDavis as I know he was talking about moving towards using more buttons and that mostly requires ghost-ui work.

@ErisDS
Copy link
Member

ErisDS commented Jul 15, 2014

Closed by #3277

@ErisDS ErisDS closed this as completed Jul 15, 2014
ErisDS pushed a commit that referenced this issue Aug 28, 2014
ErisDS pushed a commit that referenced this issue Aug 30, 2014
ErisDS pushed a commit that referenced this issue Aug 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:admin Anything relating to Ghost Admin
Projects
None yet
Development

No branches or pull requests