Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Gridview JS to respect # in filterUrl #12837

Merged
merged 2 commits into from Oct 26, 2016
Merged

Fixes Gridview JS to respect # in filterUrl #12837

merged 2 commits into from Oct 26, 2016

Conversation

cebe
Copy link
Member

@cebe cebe commented Oct 25, 2016

Q A
Is bugfix? yes
New feature? no
Breaks BC? no
Tests pass? n/a
Fixed issues #12836

fixes #12836

@cebe cebe added this to the 2.0.11 milestone Oct 25, 2016
@cebe cebe added the type:bug Bug label Oct 25, 2016
@cebe cebe merged commit b49ce4f into master Oct 26, 2016
@cebe cebe deleted the gridview-patch-1 branch October 26, 2016 13:08
@arogachev
Copy link
Contributor

arogachev commented Dec 17, 2016

This fix is incorrect. With these filter urls we will get the following results:

  • With filter url /posts/index#post: expected - /posts/index#post, actual - /posts/index#post/posts/index#post.
  • With filter url /posts/index?foo=1&bar=2#post: expected - /posts/index#post, actual - /posts/index?foo=1&bar=2#post.
  • With filter url #post: expected - #post, actual - #post#post.
  • With filter url ?foo=1&bar=2#post: expected - #post, actual - ?foo=1&bar=2#post.

I found this during writing of the tests for yii.gridView.js. I've already fixed it and will send PR today.

@@ -118,6 +118,10 @@

var pos = settings.filterUrl.indexOf('?');
var url = pos < 0 ? settings.filterUrl : settings.filterUrl.substring(0, pos);
var hashPos = settings.filterUrl.indexOf('#');
if (hashPos >= 0) {
url += settings.filterUrl.substring(pos);
Copy link
Contributor

Choose a reason for hiding this comment

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

hashPos must be used instead of a pos here. Also the anchor needs to be added only when get parameters was cut off, otherwise url already contains anchor and it will lead to duplication again.

@arogachev
Copy link
Contributor

Fully fixed in #13230.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gridview FilterUrl does ignores # link in URL
3 participants