Bug in pagecontaner.js (v1.4.x) #7177

Closed
srgstm opened this Issue Feb 25, 2014 · 3 comments

Projects

None yet

3 participants

@srgstm
srgstm commented Feb 25, 2014

Pagecontaner widget has _handleDestination private method. The last 'if' statement in this method contains comparison:

history.stack[0].url !== history.initialDst.replace( $.mobile.dialogHashKey, "" )

However this comparison always evaluates to true because left part refers to full URL and the right part refers to just the hash part (with the # stripped out).

@gabrielschulhof
Contributor

OK, this is part of funky-initial-URL-handling ... I'll take a look ...

@gabrielschulhof gabrielschulhof self-assigned this Feb 26, 2014
@gabrielschulhof gabrielschulhof added this to the 1.5.0 milestone Feb 26, 2014
@gabrielschulhof gabrielschulhof removed their assignment Feb 26, 2014
@gabrielschulhof
Contributor

Hmmm ... it seems that it is never evaluated in any of our tests, except in a unit test specifically designed to hit that method.

@jaspermdegroot jaspermdegroot modified the milestone: 1.4.4, 1.5.0 Jun 3, 2014
@gabrielschulhof
Contributor

In the process of removing initialDst from the code, which really is useless, I came across an interesting nav behaviour when deep-linking:

  1. Open http://jsfiddle.net/zm6kprsf/1/show/light/#/k9L6hpn8/1/show/light/ in your browser (this is a URL of the form http://domain/path1/#/path2, meaning that the page at path2 should be displayed in the end and the URL of path2 should show up in the location in the end)
  2. Click on "Page 2", which is a link to an internal page

At this point, the location will display /path1/#page2 instead of /path2/#page2. This is bad, because opening an internal page should only change the hash, not the path.

The sequence tests should've caught this but I forgot to include an assertion that checks the location :(

@arschmitz do you think this is bad enough to warrant a fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment