IE11 adds undefined to url when defining a scrollBehaviour

[nirazul]

Version

3.0.1

Reproduction link

Please read below, I wasn't able to reproduce the error in a minimum fiddle.
https://codepen.io/Nirazul/pen/dJxvZB?editors=1010

Steps to reproduce

Even after debugging a few hours, I wasn't able to manage a reproduction and I'm unsure what that means. However I've probably found the issue and enough hints to why it's occuring.

The problem is that IE11 is adding a /undefined to the url, when I'm initializing the router with a scrollBevariour method. The reason is the following line of code, which was implemented to fix #1585:

https://github.com/vuejs/vue-router/blob/dev/src/util/scroll.js#L11

There are similar issues in other projects, that have been closed:
ampproject/amphtml#743

I haven't found an easy way to fix this except commenting out the line. So the easiest fix would be to test for IE and not execute the line at all.

What is expected?

IE should not add /undefined to the route when providing a scrollBehaviour function.

What is actually happening?

IE (under some arcane circumstances) adds /undefined to the URL, when vue-router's setupScroll is executed.

---

As I said, I wasn't able to reproduce it in a reduced Fiddle. When stepping through the code, I could identify the line that was causing it. I'm not sure however, if some kind of listener may be the cause.

[posva]

Thanks for the detailed issue 😉 , always a pleasure!
Cannot give it a look right now but will come back later to see if there's something else or if it's indeed just a matter of skipping that line on IE

[nirazul]

Thanks for looking into this!

I've found out some more details:

1. It appears that MS Edge is also affected, so it's not only IE.
2. We have two environments that behave differently. One shows the bug (of course it must be production), the other one is not affected. When stepping through the code, window.history.state has differing values when the setupScroll method is invoked. The bug seems to occur only when window.history.state is null. On the other environment, there's a state like { key: '1385.24' }, which seems to be set by vue-router

[jeerbl]

Hi there! Just to let you know that this behaviour can be reproduced on IE9 as well.

[nirazul]

@jeerbl Have you, by any chance, managed to make a fiddle or do you have an example on a publicly accessible page? That would be great to work out a solution!

[jeerbl]

If you try to access demo.trackinsight.com with IE, you will be redirected to demo.trackinsight.com/undefined.

I tried removing the scrollBehavior from my router configuration and the bug disappeared. I'll load this workaround soon so this bug example won't be here anymore at the end of the day.

EDIT

Please use demo.trackinsight.com to test as www.trackinsight.com was fixed by removing the scrollBehavior from the router config (workaround until this is fixed).

[nirazul]

@jeerbl
Thanks! I could reproduce it on the demo page with IE11 and MSEdge 20.

@posva
The bug seems to be fixed in MSEdge 38. So it's basically IE and older versions of MSEdge

[posva]

Haven't gave this a try yet, but thanks! Good to know

…

On Wed, 31 Jan 2018, 09:47 Rouven Bühlmann, ***@***.***> wrote: @jeerbl <https://github.com/jeerbl> Thanks! I could reproduce it on the demo page with IE11 and MSEdge 20. @posva <https://github.com/posva> The bug seems to be fixed in MSEdge 38. So it's basically IE and older versions of MSEdge — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2023 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoicdSZx5e1s7H9o6mOn3f2DgRMyOHgks5tQCizgaJpZM4Ruhlz> .

-- Eduardo San Martin Morote

[posva]

@nirazul Sorry for the delay. I think the best for now is not to do is to not execute that line if we are on IE. Maybe we should even do it only for Firefox. Feel free to fire the PR as you are the one who did the research 😉

[nirazul]

@posva I've looked into it again and made some more observations:
The issue definitely occurs when window.history.state has the value null and window.history.replaceState({} /* or '' */) is executed.

The funny thing is: window.history.replaceState has been patched:
Patched log: function(b,f,g){e.apply(d,[b,f,g]);c({source:a,state:b,j:db(ca())})}
Unpatched log: function replaceState() { [native code] }

So this means that some library patches the history API for some browsers but introduces a bug.

[nirazul]

Ok, so I've found the culprit. The issue is created by the GTM (Google Tag Manager) script. It replaces native history APIs in IE/Edge and introduces the bug.

There's a report on a google forum:
https://productforums.google.com/forum/#!topic/tag-manager/uG6FtPInRGs;context-place=forum/tag-manager

The author also mentions a fix:

// Fix issue with GTM overwriting replaceState, causing url changes in IE and Edge browsers.

function fixHistoryMethod(name) {
    var oldMethod = window.history[name];
    window.history[name] = function(state, title, url) {
        if (url === undefined) {
            oldMethod.apply(window.history, [state, title]);
        } else {
            oldMethod.apply(window.history, [state, title, url]);
        }
    }
}

fixHistoryMethod("replaceState");
fixHistoryMethod("pushState");

[posva]

Thanks a lot for posting the fix back!

[jimbatamang]

Users said, google already fixed it:
https://productforums.google.com/forum/#!topic/tag-manager/uG6FtPInRGs;context-place=forum/tag-manager

But we're still having this issue. Any idea?

[nirazul]

@jimbatamang
What do you get when you execute window.history.replaceState in the faulty browser? I haven't tested yet if the patch is not necessary anymore.

[dobromir-hristov]

We dont have GTM on our page and this still is an issue.

Removing the scrollBevariour functionality removed the problem.

[jeerbl]

Any news on this?