ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.03k stars 13.51k forks source link

Keeping url parameters with deeplinking #12328

Closed stevengunneweg closed 5 years ago

stevengunneweg commented 7 years ago

Ionic version: [ ] 1.x [ ] 2.x [x] 3.x

I'm submitting a ... [ ] bug report [x] feature request [ ] support request

Current behavior: Currently when I have a URL with parameters (e.g. ..?a=hello&b=world) they get cut off by Ionic routing.

http://localhost:8100/#/nav/n4/intro?a=hello&b=world gets changed to http://localhost:8100/#/nav/n4/intro.

These parameters are for FB tracking and such and are necessary on all pages.

Expected behavior: http://localhost:8100/#/nav/n4/intro?a=hello&b=world should stay like that.

I would like to keep these parameters. Ideally it would be a settings parameter.

Steps to reproduce: Add ?a=hello&b=world at the end of your URL and do a refresh. The added parameters will be removed.

Related code:

Other information:

Ionic info:

global packages:

    @ionic/cli-utils : 1.5.0
    Cordova CLI      : 6.5.0 
    Ionic CLI        : 3.5.0

local packages:

    @ionic/app-scripts              : 2.0.1
    @ionic/cli-plugin-cordova       : 1.4.1
    @ionic/cli-plugin-ionic-angular : 1.3.2
    Cordova Platforms               : android 6.2.3 ios 4.4.0
    Ionic Framework                 : ionic-angular 3.5.0

System:

    Node       : v6.7.0
    OS         : macOS Sierra
    Xcode      : Xcode 8.3.3 Build version 8E3004b 
    ios-deploy : 1.9.0 
    ios-sim    : 5.0.8 
    npm        : 3.10.3 
jgw96 commented 7 years ago

Thanks for using Ionic, we will look into this. Would you be able to share a small repo we could use to reproduce this issue?

danbucholtz commented 7 years ago

@stevengunneweg,

Can you check if this happens with ionic-angular at version 3.4.0?

rm -rf node_modules/ionic-angular
npm install ionic-angular@3.4.0

Thanks, Dan

stevengunneweg commented 7 years ago

Thanks for your quick responses.

@jgw96 I don't have a small repo ready for this but you can use any ionic v3.4+ (possibly with earlier versions as well but I haven't tested this myself) project to test this.

@danbucholtz I did see the same behavior with v3.4.0. I updated to v3.5.0 hoping this would've been resolved by the changes but the issue is still there.

danbucholtz commented 7 years ago

@stevengunneweg, okay cool, I am just glad I didn't break anything 😸

We'll get it fixed ASAP.

Thanks, Dan

lincolnthree commented 7 years ago

Confirmed, I am also struggling with this now on v3.4.0 and have been looking into it for a few days -- I believe the problem lies in the fact that deep-linker.ts actually includes URL search/query parameters (window.location.search) in its view calculation. Based on the fact that @IonicPage only allows specification of the path segment, I would view this as a bug -- the search string should not be used to determine resource(view) location. This generally violates common practice for how URL search parameters are most commonly used:

https://github.com/ionic-team/ionic/blob/master/src/navigation/deep-linker.ts#L56

If Ionic isn't currently using search parameters to determine navigation outcomes, and if you don't plan on it (or even if you are) a short-term solution should be as simple as stopping URL-matching/normalization when you reach the query string, here:

https://github.com/ionic-team/ionic/blob/master/src/navigation/deep-linker.ts#L442

If you need to support the hash, then you can begin again at the first instance of the '#' character (strip the search terms and leave the hash):

/**
 * @param {?} browserUrl
 * @return {?}
 */
export function normalizeUrl(browserUrl) {
    browserUrl = browserUrl.trim();
    if (browserUrl.charAt(0) !== '/') {
        // ensure first char is a /
        browserUrl = '/' + browserUrl;
    }
    if (browserUrl.length > 1 && browserUrl.charAt(browserUrl.length - 1) === '/') {
        // ensure last char is not a /
        browserUrl = browserUrl.substr(0, browserUrl.length - 1);
    }
    console.log("Normalized: ", browserUrl)
    let truncated = browserUrl.replace(/\?[^#]+/, "");
    console.log("Truncated: ", truncated)
    return truncated;
}

This prevents the search string (wherever it appears) from being used in calculation of the Nav URL state. (or whatever you call it.) The only issue I see with my "hack" is if you for some reason no search string exists in the URL, you embedded a '?' character in the hash itself (and did not intend for it to represent a query string), but that can easily be fixed with a little more regex-fu.

A little history about me: I created http://www.ocpsoft.org/rewrite/ and http://www.ocpsoft.org/prettyfaces/, as well as serving on the Java EE: Java Server Faces Expert Group focusing on URL navigation and page parameters -- I'm happy to try to provide any feedback I can about URL/navigation features if you'd like. My only issue is that I'm not a JS/TS/Angular expert -- still learning at this point.

But... supporting proper URL-based navigation is essential for web application adoption (which is how I am using Ionic). In fact, I can't upgrade to 3.5.0 because the URL structure breaks my web application, and I'm really hoping this doesn't stay the case.

lincolnthree commented 7 years ago

A footnote. Once you make this tweak, you can use Angular's Location to change the search string without breaking Ionic's Navigation stack:

let query = new URLSearchParams(this._location.path.replace(/^([^?]*)?\??/, ''));
// ...
let path = this._location.path(true);
path = path.replace(/^([^?]*)?.*/, '$1');
this._location.replaceState(path, query.toString().replace(/^\?/, ""));
lincolnthree commented 7 years ago

You can see this all working here: https://www.topdecked.me/?lat=34.95123763552474&long=-80.87257438437501&z=8&d=118&sdt=2017-07-11T04:00:00.001Z&format=Legacy

lincolnthree commented 7 years ago

Ah, sorry, you wanted to preserve the parameters. Sorry, I misunderstood. Some of this may still be applicable/related, but I'm thinking I should probably move this to a new issue.

tattivitorino commented 7 years ago

Hello everyone! I have just moved to the lazy-loading and IonicPage Modules etc.. It is quite nice indeed, but there's one thing that's getting on my "nerves"!! the url's! My app has the basic interfaces! A LoginPage, SignupPage... that after login or signup it goes to the tabs interface.. nothing fancy i guess! First of all the "nav/n4" is it default behaviour? Am I missing something? and when I get to the Tabs it gets even worse.. "/nav/n4/app/tabs/t1/perfil/perfil where PerfilPage is the first for the tabs interface. I created the pages and the Tabs with ionic g and for all the pages i have the segment set for their, let's say, names like:

@IonicPage({ segment:'perfil' })

I'm certainly missing something!

Here's my system info:

`global packages:

@ionic/cli-utils : 1.4.0
Ionic CLI        : 3.4.0

local packages:

@ionic/app-scripts              : 1.3.12
@ionic/cli-plugin-ionic-angular : 1.3.2
Ionic Framework                 : ionic-angular 3.5.0

System:

Node       : v6.5.0
OS         : OS X El Capitan
Xcode      : Xcode 8.1 Build version 8B62 
ios-deploy : 1.9.1 
ios-sim    : 5.0.13 
npm        : 3.10.3 

`

Any help will be very much appreciated! Cheers

stevengunneweg commented 7 years ago

@tattivitorino I see this as well but I think this should be on a different thread. When you do this, please tag me in it since I'm also very interested in this. To add to your comment, when I reload on a page similar to /nav/n4/app/tabs/t1/perfil/perfil I get redirected to my rootPage (as defined in app.component.ts. Do you see this behaviour as well?

Update This url behaviour is only since Ionic v3.5.0. v3.4.0 had nicer urls and no redirect issues as far as i've seen.

tattivitorino commented 7 years ago

@stevengunneweg I'll create a new thread then! thanks

I couldn't tag you in the thread I created but here's the link: https://github.com/ionic-team/ionic/issues/12346

lincolnthree commented 7 years ago

Hmm, I'm not exactly sure. I haven't been able to upgrade to 3.5.0 due to the URL changes, but I don't seem to have a problem with reloading. The issue I do have with reloading is that the back button breaks after you hard refresh the app.

stevengunneweg commented 7 years ago

@jgw96 @danbucholtz is there any update on this? I upgraded to ionic-angular: "3.5.3" but url parameters are still removed on refresh/navigation in the browser (not using ionic-labs). I really need the parameters to stay in the url.

I looked at the comments of lincolnthree but I'm not comfortable with making changes in the Ionic framework files.

global packages:

@ionic/cli-utils : 1.5.0
Cordova CLI      : 6.5.0 
Ionic CLI        : 3.5.0

local packages:

@ionic/app-scripts              : 2.0.2
@ionic/cli-plugin-cordova       : 1.4.1
@ionic/cli-plugin-ionic-angular : 1.3.2
Cordova Platforms               : android 6.2.3 ios 4.4.0
Ionic Framework                 : ionic-angular 3.5.3

System:

Node       : v6.7.0
OS         : macOS Sierra
Xcode      : Xcode 8.3.3 Build version 8E3004b 
ios-deploy : 1.9.0 
ios-sim    : 5.0.8 
npm        : 3.10.3 
lincolnthree commented 7 years ago

@stevengunneweg It's a one-line change that I've been testing for a week or so now. Your hesitance to make changes to framework files is totally understandable. The risk of "forgetting" to keep the change in sync with new updates is enough for me to push back on it, but unfortunately this is a critical requirement of my app and I have no other choice. I'd love to see this addressed in the framework so I don't have to make sure my devs keep this change in sync with every new install or refresh.

lincolnthree commented 7 years ago

Here's a PR with the change, for convenience.

https://github.com/ionic-team/ionic/pull/12399

danbucholtz commented 7 years ago

@lincolnthree,

I did take a look at your PR and we'll get it in for the next (not today's) Ionic release.

Did this ever work correctly with Ionic in the past?

Thanks, Dan

danbucholtz commented 7 years ago

@lincolnthree,

Are you on Ionic worldwide slack? If not, can you sign up and DM me? Let's take this over the line. I noticed the query params disappear after changing page with your PR. I am not sure if that is a good behavior or not. Let's chat about it and get the PR merged. My username is danb.

Thanks, Dan

Manduro commented 7 years ago

I'm wondering if there is an actual issue here. @stevengunneweg you mention using http://localhost:8100/#/nav/n4/intro?a=hello&b=world, while you should be using http://localhost:8100/?a=hello&b=world#/nav/n4/intro as far as I know. Or am I missing something and is there an actual use case for using these parameters in the url fragment itself?

Quoting wikipedia here (don't tell anyone I did), emphasis mine:

In URIs a hash mark # introduces the optional fragment near the end of the URL. The generic RFC 3986 syntax for URIs also allows an optional query part introduced by a question mark ?. In URIs with a query and a fragment, the fragment follows the query.

When I run the conference demo app with lazy loading enabled, then open http://localhost:8100/?param=hi#/tabs-page/conference-schedule/schedule and navigate around, the parameter stays as I think is to be expected and correct behaviour.

lincolnthree commented 7 years ago

Honestly if the parameter does stay around during navigation, that seems mildly problematic to me. Though I understand the technical reasons why it would if you are using the hash # for navigation. Your application should have no impact on the search string whatsoever.

Using pushState for navigation however, I would expect the opposite. The search / query string should disappear when navigating unless the programmer has explicitly requested that the parameters be preserved or passed between navigation events.

lincolnthree commented 7 years ago

Also @danbucholtz, no, this has never worked previously in Ionic to my knowledge (though I have never tried in prior versions, but I can't see how it would have.)

stevengunneweg commented 7 years ago

@Manduro for me this is indeed an actual issue. Placing the parameters before the hashtag works indeed, however when I use locationStrategy: 'path' (which is the case on our live environment) the parameters are stripped off again, no matter their position.

Manduro commented 7 years ago

Well as @lincolnthree mentions that sounds like expected behaviour. I have yet to see a website or application that persists URL parameters across all pages (save the old phpsessid's).

What are you using these parameters for? Wouldn't it be a better option to save them in local storage or a cookie for example? Those are methods commonly used for persistent data, unlike URL parameters.

stevengunneweg commented 7 years ago

Hey @Manduro, these parameters are added externally for tacking (Facebook for example). I have no control over these.

Manduro commented 7 years ago

Most of the time tracking parameters like that are picked up by the trackers on your page (like Google Analytics, Facebook Pixel, etc), and are only required on the entry page. Subsequent navigations do not include those parameters in the url, but they are tracked none the less by the included tracker scripts.

So the entry url would be something like /tabs-page/conference-schedule/schedule?utm_source=fb, the tracker picks up that parameter and keeps track of it on subsequent navigations, although the url would not include them anymore: /tabs-page/conference-schedule/event/5. And this already works in Ionic if I'm correct! 🎉

PunkHaz4rd commented 7 years ago

Tracking is one thing, having optional parameters is another. If Ionic wish to also be able to build progressive web app, it'll need this kind of feature as well as referencing those query-params in the NavParams service.

bogomips commented 6 years ago

Building a PWA, I am still having problems to get it working, any news?

hasanlussa commented 6 years ago

I have this problems on 3.9.2 too

kevinsalerno commented 6 years ago

@lincolnthree I'm looking for authorization behavior (?token=abc) much like your coordinates params.

Which life-cycle do you hook into the app to grab/run the coordinates? Was it the string play on _location or was there an Ionic provider to grab these?

Thanks mate

lincolnthree commented 6 years ago

Hey @kevinsalerno, basically I just used the Angular framework code like such:

Get the Location object from Angular:

import { Location } from '@angular/common';
...
constructor param:
        private _location: Location,

Then access it and clean it up a bit to get just the query string.

    private getQuery() {
        let path = this._location.path(true);
        return new URLSearchParams(path.replace(/^([^?]*)?\??/, ''))
    };

Then you can read or modify the query string as desired:

            let query = this.getQuery();
                let lat = Number.parseFloat(query.get("lat"));
                let long = Number.parseFloat(query.get("long"));
                let dist = Number.parseFloat(query.get("d"));
        query.set("lat", this.coords.latitude.toString());
        query.set("long", this.coords.longitude.toString());
        query.set("z", this.zoom.toString());

Then navigate manually:

        let path = this._location.path(true);
        path = path.replace(/^([^?]*)?.*/, '$1');
        this._location.replaceState(path, query.toString().replace(/^\?/, ""));

Hope this helps!

lincolnthree commented 6 years ago

Note, you still need my patch to make this work reliably during navigation / respect changes to the parameters / pass them along, etc.

jhavinay commented 6 years ago

I just encountered this, and experimented a bit. It seems query strings gets dropped when locationStrategy: 'path'. query strings are retained when locationStrategy: 'hash' and specified correctly, i.e., "protocol://host/?query#hash

Looking forward to a speedy solution.

lincolnthree commented 6 years ago

I don't believe you'll get a speedy solution, as this issue has been open for seven months, but here's to hoping.

jhavinay commented 6 years ago

@lincolnthree,@danbucholtz , Have a look at the above PR and see if I've missed a case. This was my first look at the ionic code, so might have missed some use case. I tested it with my app, and it seems to work fine in my environment.

venkateshwarv commented 6 years ago

Another problem because of this behavior is that a new page is inserted in the navigation stack. (One with params and another without params). You will be able to see the stack in chrome if you long press the back button. Any workaround to avoid this?

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! We have moved the source code and issues for Ionic 3 into a separate repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

Thank you for using Ionic!

ionitron-bot[bot] commented 5 years ago

Thanks for the issue! We have moved the source code and issues for Ionic 3 into a separate repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

Thank you for using Ionic!

ionitron-bot[bot] commented 5 years ago

Issue moved to: https://github.com/ionic-team/ionic-v3/issues/21