Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

deep linking with query parameters via the page-params plugin no longer working in 1.1.1 #4777

Closed
mruffolo opened this Issue · 30 comments

6 participants

@mruffolo

It seems the changes described as
"Init: Proceed to hashchange handler when the initial hash contains a path" re: "34c4811"
throws a syntax error if the deep link contains parameter argument such as http://localhost/#page?arg1=value
The error is "Syntax error, unrecognized expression: ?arg1=value"

@johnbender

@mruffolo

Most likely the hash is being passed directly to a selector without being sanitized. Hopefully this is a quick fix. I'll take a look.

@johnbender

@mruffolo

We're having trouble reproducing the issue. Can you put together a jsbin or jsfiddle for us so we can work with that?

https://github.com/jquery/jquery-mobile#issues

@gabrielschulhof
Collaborator

@mruffolo

Can you please create a jsbin example showing the breakage? I can't seem to reproduce it by playing around with the documentation URLs.

@mruffolo

Crap...I see the issue and I don't think its a bug per se, so I'd first like to apologize for submitting the issue in the first place and thank you for looking at it so quickly.
What is going on is that I have a single page application and a majority of my site is dynamically generated as I get data from a RESTful webservice and when I'm dynamically generating the URL's for the different jqm 'pages' I'm setting a query parameter for the URI link that will need to be used in a subsequent ajax call to the webservice for the page data. Early on I found that I couldn't use a URI with with the forward slashes as a query parameter, e.g. "http://localhost/#page1?arg1=/uri/info/stuff" because it seems jqm treats the argument as a url and attempts load the page directly. SO, the wonderfully hacky solution was to replace all the forward slashes with commas. As I mentioned in the original issue, I'm using the page-params plugin, so when I read the parameters after pageshow, I was simply replacing the commas with forward slashes, which seemed to be fine until 1.1.1 and after some further research (10 minutes ago) it seems commas are a reserved character per the URI spec. If you guys have any clever solutions, I'm all ears.

If you want to see whats going on browse to the URL below and then refresh the page...the error will occur
http://jquerymobile.com/demos/1.1.1/docs/pages/multipage-template.html#two?arg1=,1
If you remove the comma, browse to the page again and then refresh, it will work fine.

Thanks again.

@johnbender

@mruffolo

Why don't you encode and decode the uri you're passing as the parameter value

var url = "/foo/bar", 
    idAndQueryString = "#two?foo=" + window.encodeURI( url );

$.mobile.changePage( idAndQueryString );

Then when you can use decodeURI on the other end to know where to grab the data.

@mruffolo

@johnbender
Great idea and embarrassingly obvious now...I'll give it a shot using "(de|en)codeURIComponent" because it also handles special characters , / ? : @ & = + $ #

Guess this issue can be closed.

Thanks.

@johnbender

@mruffolo

Post back if you still have issues. It's possible we still have a bug.

@mruffolo

@johnbender
Seems your idea for using encode/decodeURIComponent works in chrome and IE but not FF.
Firefox likes to automatically decode the url in the address bar thus making a refresh of the page impossible. It seems to only apply the decoding when an encoded character is found, this includes removing the hash which identifies which page jqm should navigate to.
Try putting this url in both Firefox and Chrome/IE and you can see what I mean

http://jquerymobile.com/demos/1.1.1/docs/pages/multipage-template.html#two?arg1=%2Ffoo%2Fbar

In Firefox you end up with: "http://jquerymobile.com/demos/1.1.1/docs/pages/two?arg1=/foo/bar" which is garbage.

@mruffolo

Looks like this is a bug in Firefox where they store the decoded value in location.hash
https://bugzilla.mozilla.org/show_bug.cgi?id=483304
Is there anyway jqm can work around this?

@gabrielschulhof
Collaborator

Instead of using location.hash, try this: $.mobile.path.parseUrl( location.href ).hash

This is how we get around the FF issue internally. It turns out that FF leaves location.href alone, but auto-decodes location.hash.

So, if you use jQuery Mobile's URL parsing to get at the hash from the href, you'll get the hash undecoded.

If this works for you, feel free to close the issue.

@mruffolo

@gabrielschulhof
I don't think the issue is with my code....

If you look at my comment above which states:
"Try putting this url in both Firefox and Chrome/IE and you can see what I mean
http://jquerymobile.com/demos/1.1.1/docs/pages/multipage-template.html#two?arg1=%2Ffoo%2Fbar

In Firefox you end up with: "http://jquerymobile.com/demos/1.1.1/docs/pages/two?arg1=/foo/bar" which is garbage."

Per your last comment, does that mean that location.hash shouldn't be used anywhere in the jqm codebase?

@mruffolo

@gabrielschulhof
OK, just as a quick test, I went through jqm 1.1.1 and replaced every place where location.hash was being read with $.mobile.path.parseUrl( location.href ).hash and it seems to solve the issue I'm describing with Firefox. I know there needs to be further investigation but at a high level isn't that the point of using the parseUrl function, to work around the Firefox issue?

@gabrielschulhof
Collaborator

That is exactly the point.

@mruffolo

@gabrielschulhof
@toddparker
OK, Todd, should a new issue be opened or will this suffice? It seems that someone needs to make sure that all of the places where "location.hash" is being read should be replaced with "$.mobile.path.parseUrl( location.href ).hash"

@johnbender

@toddparker @gabrielschulhof @mruffolo

This issue was corrected in master, and I've cherry picked over the corrections to fix the problem with firefox's autodecoding of the hash.

@mruffolo

You can give this a try by using the latest from our 1.1-stable branch

<script src="http://jquerymobile.com/branches/1.1-stable/js/"></script>
@johnbender

As a side note there will be another updated to similar bits of code later today to address some of the older webkit browsers auto decoding location.href.

@mruffolo

@johnbender
@toddparker
This still isn't working....in Firefox the URL displayed in the address bar is still displaying decoded and missing the hash (#)...so when you refresh on the URL, the page doesn't reload.

There were still a handful of places that were using the location.hash.

I see there is a a second "new" way to work around this issue that was implemented, but I don't think its fully solving this problem and I don't quite understand why.

Going back to an earlier post....I got this working myself in the 1.1.1 release by simply just changing everywhere that location.hash was being read with $.mobile.path.parseUrl( location.href ).hash

I also went through the stable 1.1.1 release and changed every location.hash, $.mobile.path.parseLocation().hash and path.parseLocation().hash with $.mobile.path.parseUrl( location.href ).hash and its working for me.

Throughts?

@johnbender

@mruffolo

Can you whip up a jsbin or jsfiddle that reproduces the issue in firefox. I'd like to see if we can avoid changing the @cowboy's hashchange plugin and there's only one reference to location.hash left in navigation.

Thanks

@mruffolo

@johnbender

Not sure how I can show this with jsfiddle being that its only reproducible when URL is changed...maybe I'm not familiar enough with jsfiddle to accomplish this, I'll see if I can come up with something. Though, as I've stated a few times earlier, you can see my issue by putting this URL in Chrome/IE and then in Firefox....it doesn't work in Firefox
URL:
http://jquerymobile.com/demos/1.1.1/docs/pages/multipage-template.html#two?arg1=%2Ffoo%2Fbar

Regarding the location.hash only having one more reference, I just found the following areas in the 1.1 stable release where I think the location.hash should be changed to use the $.mobile.path.parseUrl( location.href ).hash function. I think that changing all of these as well as all the places where parseLocation().hash is being used fixes the issue for me. I'm hoping someone can explain why these changes aren't a good idea?

//get path from current hash, or from a file path
            get: function( newPath ) {
                if( newPath === undefined ) {
                    newPath = location.hash;
                }
                return path.stripHash( newPath ).replace( /[^\/]*\.[^\/*]+$/, '' );
            }
init: function() {
            $win.bind( "hashchange", self.onHashChange );

            // Handle popstate events the occur through history changes
            $win.bind( "popstate", self.onPopState );

            // if there's no hash, we need to replacestate for returning to home
            if ( location.hash === "" ) {
                history.replaceState( self.state(), document.title, $.mobile.path.getLocation() );
            }
        }
// Auto-add back btn on pages beyond first view
            if ( o.addBackBtn &&
                role === "header" &&
                $( ".ui-page" ).length > 1 &&
                $page.jqmData( "url" ) !== $.mobile.path.stripHash( location.hash ) &&
                !leftbtn ) {

                backBtn = $( "<a href='javascript:void(0);' class='ui-btn-left' data-"+ $.mobile.ns +"rel='back' data-"+ $.mobile.ns +"icon='arrow-l'>"+ o.backBtnText +"</a>" )
                    // If theme is provided, override default inheritance
                    .attr( "data-"+ $.mobile.ns +"theme", o.backBtnTheme || thisTheme )
                    .prependTo( $this );
            }
// if hashchange listening is disabled, there's no hash deeplink,
            // the hash is not valid (contains more than one # or does not start with #)
            // or there is no page with that hash, change to the first page in the DOM
            // Remember, however, that the hash can also be a path!
            if ( ! ( $.mobile.hashListeningEnabled &&
                     $.mobile.path.isHashValid( location.hash ) &&
                     ( $( location.hash + ':jqmData(role="page")' ).length ||
                       $.mobile.path.isPath( location.hash ) ) ) ) {
                $.mobile.changePage( $.mobile.firstPage, { transition: "none", reverse: true, changeHash: false, fromHashChange: true } );
            }
            // otherwise, trigger a hashchange to load a deeplink
            else {
                $window.trigger( "hashchange", [ true ] );
            }
@johnbender

Creating two jsbin's normally works.

@mruffolo

@johnbender @toddparker
Did you even try the url I provided in Firefox? It demonstrates the issue perfectly.

@mruffolo

@johnbender @toddparker

I don't see how this is possible to do in jsbin/jsfiddle

I wrote a really simple app to demonstrate the issue:
1. Using Firefox, browse to this URL: http://concord.jhuapl.edu/jqm/
2. Click button "Go To Page 2"
3. After Page 2 is displayed......Hit F5 (refresh)
4. Page doesn't load

Try this in IE or Chrome and it works fine.

@mruffolo

@johnbender
Any more thoughts on this?

@gabrielschulhof
Collaborator

@mruffolo

I tried the above URL and all I get is "Connecting..."

@mruffolo

@gabrielschulhof
Not sure what the issue is, its working for me. Maybe its because we have a self-signed cert, this server is a bit of a sandbox area for us at my office.

Try this URL instead.... http://home.comcast.net/~mruffolo02/

@jaspermdegroot
Collaborator

Is this still an issue with latest code?

@mruffolo

@uGoMobi
Yes, this is still an issue per 1.3.1
It would be a shame if the official stance was deep linking with query parameters are officially not supported, that's a major hindrance to being able to use this framework if you're dynamically generating page content based upon discoverable URL's via a RESTful webservice.

@arschmitz
Owner

closing this issue the official stance has always been this is not supported and we have decided that as of 1.5 we will allow any valid id for embded pages this means ? is a valid character for an id so the params will stay in link however they will not be treated as query params they will be treated as part of the ID

@arschmitz arschmitz closed this
@CameronAskew

@mruffolo I know this is an old issue but I built a plug-in for jquery mobile 1.4+. I could probably easily add support for 1.3 and maybe even 1.2 if people needed it.

https://github.com/CameronAskew/jquery.mobile.paramsHandler

@mruffolo

@CameronAskew thanks for the info, but we've since abandoned the use of jqm because of this and a few other reasons. If we ever consider going back, I'll keep this in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.