browser icon indicating copy to clipboard operation
browser copied to clipboard

clicking links (<a> elements) with href omitted or with an empty href incorrectly generates External UrlRequest's

Open glennsl opened this issue 7 years ago • 6 comments
trafficstars

Empty links, i.e. a elements without a href attribute or with href "" will cause External UrlRequests to be generated, and if handled as suggested in the Elm guide will cause page reloads.

This can be handled in update if you know about it, but is non-standard behavior that I think would surprise most people. it is also behavior that a lot of code depends on, among them elm-bulma, and it miight therefore not be immediately apparent what the cause is.

As far as I can understand, the standard behavior is:

  • When href is omitted it should not be considered a hyperlink (source)
  • When href is an empty string, it should be considered a reference to the beginning of the document (source)

My suggestion is to simply not emit a UrlRequest if href is omitted, and to emit an Internal UrlRequest if it is an empty string.

Here's a minimal, complete and verifiable example:

module Main exposing (main)

import Browser
import Browser.Navigation
import Html exposing (..)
import Url


type alias Model =
    ()


type Msg
    = UrlRequested Browser.UrlRequest
    | UrlChanged Url.Url


init () _ _ =
    ( (), Cmd.none )


update msg model =
    case msg of
        UrlRequested (Browser.Internal _) ->
            ( model, Cmd.none )

        UrlRequested (Browser.External url) ->
            ( model, Browser.Navigation.load url )

        UrlChanged _ ->
            ( model, Cmd.none )


view model =
    { title = ""
    , body = [ a [] [ text "click to reload" ] ]
    }


main =
    Browser.application
        { init = init
        , view = view
        , update = update
        , subscriptions = \_ -> Sub.none
        , onUrlRequest = UrlRequested
        , onUrlChange = UrlChanged
        }

My solution/workaround is to add a branch in update to do nothing on External "". For others coming across this looking for an easy fix, here's the fixed update function:

update msg model =
    case msg of
        UrlRequested (Browser.Internal _) ->
            ( model, Cmd.none )

        UrlRequested (Browser.External "") ->
            ( model, Cmd.none )

        UrlRequested (Browser.External url) ->
            ( model, Browser.Navigation.load url )

        UrlChanged _ ->
            ( model, Cmd.none )

glennsl avatar Oct 08 '18 19:10 glennsl

As far as I can tell, this is how things work in plain HTML:

  • clicking <a>test</a> does nothing (a.href === undefined)
  • clicking <a href="">test</test> is like refreshing the page (a.href === location.href)

lydell avatar Oct 08 '18 19:10 lydell

clicking <a href="">test</test> is like refreshing the page (a.href === location.href)

Hmm, indeed. I'm not able to read that from the spec(s), though it seems Elm kind of accidentally does the right thing then. And while it would still seem technically correct to generate an Internal UrlRequest, perhaps that would just make it more cumbersome to get the expected behaviour. I'm not even sure what I'd consider expected behavior in an SPA though.

glennsl avatar Oct 08 '18 20:10 glennsl

This was reported on browser already: https://github.com/elm/browser/issues/34

MethodGrab avatar Oct 10 '18 16:10 MethodGrab

Ah, thanks @MethodGrab! And a proposed fix is in elm/virtual-dom, so it seems the issue is well-covered.

I'd close this if it weren't for the issue with empty (as opposed to omitted) hrefs that's not covered by that issue and fix. I'll happily migrate this issue to a more suitable project though.

glennsl avatar Oct 10 '18 16:10 glennsl

I just ran in to this too, in the context of an a tag with an onClick attribute but no href specified at all. A possible fix, provided you aren't relying on fragment identifiers in the same context, is to add a href "#" attribute to the tag. This will trigger a URL that will be correctly recognized as internal.

davcamer avatar Dec 06 '18 20:12 davcamer

In the mean-time, a docs update would be highly appreciated.

robx avatar Apr 03 '19 06:04 robx