Skip to content

Commit

Permalink
Fixed jquery-archive#2234 - form hijacking wasn't respecting allowCro…
Browse files Browse the repository at this point in the history
…ssDomainPages, now it does!
  • Loading branch information
adrianpike committed Nov 16, 2011
1 parent ff93c76 commit 88754ac
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions js/jquery.mobile.navigation.js
Expand Up @@ -1256,8 +1256,10 @@

url = path.makeUrlAbsolute( url, getClosestBaseUrl($this) );

//external submits use regular HTTP
if( path.isExternal( url ) || target ) {
// More info about what's going on here is up in useDefaultUrlHandling in the Click routing.
// Basically if we loaded via file:// and we've got "allowCrossDomainPages" true, we should use changePage.
isCrossDomainPageLoad = ( $.mobile.allowCrossDomainPages && documentUrl.protocol === "file:");

This comment has been minimized.

Copy link
@johnbender

johnbender Dec 5, 2011

I'm under the impression that the allowCrossDomainPages attribute was put in place to handle CORS requests but the variable name and the logical and operator seem to suggest that cross domain page loads are confined to those made with the file protocol. Was that the intention?

if(( path.isExternal( url ) && !isCrossDomainPageLoad) || target ) {
return;
}

Expand Down

1 comment on commit 88754ac

@adrianpike
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - I'm basically using the same functionality @jblas put in place under the click handler.
(Better comment here:

// Some embedded browsers, like the web view in Phone Gap, allow cross-domain XHR
)

Looks like this was originally set up in 2aab30b

I'd be definitely fine with making it wide open if allowCrossDomainPages is true, but I didn't want to do anything too dangerous, so I stuck with just bringing the clickhandler stuff over for forms. :)

Please sign in to comment.