Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix for issue 4882 #5221

Merged
merged 5 commits into from Oct 26, 2012

Conversation

Projects
None yet
2 participants
Contributor

jefflembeck commented Oct 25, 2012

Changed the url parser's regular expression to eliminate any leading white space. Browsers ignore this space, so this is expected behavior. With the latest, it will create a 404. Before that, it would cause the next page to load without ajax.

This fixes issue #4882.

jefflembeck added some commits Oct 25, 2012

@jefflembeck jefflembeck Changed urlParseRE to ignore space at beginning. This is expected
behavior in browsers. This used to result in pages changing to
"%20destination.html" instead of the now "destination.html". Fixes issue #4882
0c44912
@jefflembeck jefflembeck Merge branch 'master' of github.com:jquery/jquery-mobile into issue-4882
Conflicts:
	js/jquery.mobile.navigation.js
fa76784
Member

gseguin commented Oct 25, 2012

@jlembeck, thanks for the PR.

That test passes without your patch to the regexp in Chrome 22 / OSX. Could you provide a test that fails with the code on master?

Contributor

jefflembeck commented Oct 26, 2012

This test fails without the code change and passes with it on Chrome 22/ OS X. Thanks @gseguin

@gseguin gseguin and 1 other commented on an outdated diff Oct 26, 2012

tests/unit/navigation/navigation_core.js
@@ -1307,4 +1307,18 @@
}
]);
});
+
+ asyncTest( "external page is accessed correctly even if it has a space in the url", function(){
+ $.testHelper.pageSequence([
+ function(){
+ $.mobile.changePage( " external.html" );
+ },
+ function(){
+ ok( $.mobile.activePage.attr( "id" ) === "external-test", "the correct page is loaded" );
@gseguin

gseguin Oct 26, 2012

Member

Isn't equal more appropriate than ok here?

@jefflembeck

jefflembeck Oct 26, 2012

Contributor

Agreed. Change made.

@gseguin gseguin added a commit that referenced this pull request Oct 26, 2012

@gseguin gseguin Merge pull request #5221 from jlembeck/master
Fix for issue 4882
b88b6f3

@gseguin gseguin merged commit b88b6f3 into jquery:master Oct 26, 2012

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