Opened 14 years ago
Closed 13 years ago
#16549 closed defect (bug) (fixed)
Plugin searches maintains activated/deactivated URL parameters
Reported by: | kawauso | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.3 | Priority: | normal |
Severity: | normal | Version: | 3.1 |
Component: | Administration | Keywords: | has-patch commit 3.3-early |
Focuses: | Cc: |
Description
Steps to reproduce:
- Activate a plugin
- Search for anything
- A status message for the activation status will still appear
Attachments (6)
Change History (28)
#3
@
14 years ago
Will that patch preserve any filters applied? (for example, Inactive plugins only?)
If not, we can utilise $_SERVER['REQUEST_URI']
somehow as that'll have any action-based queryvars striped from it already.
#4
follow-up:
↓ 5
@
14 years ago
@dd32 no, but these filters were not preserved even before. Search results always show both inactive/active plugins.
#5
in reply to:
↑ 4
@
14 years ago
Replying to solarissmoke:
@dd32 no, but these filters were not preserved even before. Search results always show both inactive/active plugins.
This is unfortunately true. In 3.0.x, filters are not preserved.
I personally think filters should be preserved.
#6
follow-up:
↓ 7
@
14 years ago
I personally think filters should be preserved.
Filters are currently preserved for the next request, It's just that the Search does not abide by the filters.
Replace esc_attr() with esc_url() and 16549.3.diff should be right to go, Needs another devs thoughts however
#7
in reply to:
↑ 6
;
follow-up:
↓ 16
@
14 years ago
Replying to dd32:
I personally think filters should be preserved.
Filters are currently preserved for the next request, It's just that the Search does not abide by the filters.
That's what I meant. In 3.0.x, the filters are not preserved when you search. So not preserving filters when searching is not a regression, and fixing it would be an enhancement (which probably means a separate ticket).
solarissmoke's patch will redirect a search executed in wp-admin/network/plugins.php
to wp-admin/plugins.php
. Plus there might be unexpected consequences when performing bulk action (paging being reset to 1 for example).
If this issue described in the ticket is considered a regression (which I think it is, albeit a minor one), restoring the search behavior in 3.0.x is probably the safest route to take. See 16549.4.diff .
#8
follow-up:
↓ 9
@
14 years ago
16549.4.diff
Doesnt work, Has exactly the same problems as current, except it doesnt include the filters in the url, You'd have to include REQUEST_URI in order to fix the actual issue here.
#9
in reply to:
↑ 8
@
14 years ago
Replying to dd32:
16549.4.diff
Doesnt work, Has exactly the same problems as current, except it doesnt include the filters in the url, You'd have to include REQUEST_URI in order to fix the actual issue here.
I tested it and the problem described in the ticket is gone after applying 16549.4.diff. Because method="get"
, all the current filters as well as status vars are cleared out when searching.
#10
@
14 years ago
To make it perfectly clear for reviewers, 16549.4.diff attempts to fix the bug outlined in the ticket description (and restore the behavior to that of 3.0.x), rather than attempting to preserve the filters when searching as well (which I think should be a separate enhancement ticket for 3.2).
If you want to restore the filters when searching, as dd32 said, 16549.3.diff is good to go with a small modification.
#11
follow-ups:
↓ 12
↓ 13
@
14 years ago
To make it perfectly clear for reviewers, 16549.4.diff attempts to fix the bug outlined in the ticket description (and restore the behavior to that of 3.0.x), rather than attempting to preserve the filters when searching as well (which I think should be a separate enhancement ticket for 3.2).
And like I said, It doesn't actually fix anything. It uses the same style as current in core(<form method="get" action="">
) which results in the activation flags present in the current url being carried between requests.
I don't care about keeping the filters, but since it doesn't work now, it doesn't matter that it doesn't work afterwards, that can be left for 3.2 to decide what to do there.
I just want a minimal change which fixes the current bug :)
#12
in reply to:
↑ 11
@
14 years ago
Replying to dd32:
And like I said, It doesn't actually fix anything. It uses the same style as current in core(
<form method="get" action="">
) which results in the activation flags present in the current url being carried between requests.
Hmm, that's strange, because it works for me. I tested the patch several times since your previous comments and it seems to fix the problem.
Changing method
to get
on the search form doesn't pass along activation flags, in my case, at least on Chrome and Firefox.
Could you upload a screenshot somewhere?
#13
in reply to:
↑ 11
@
14 years ago
Replying to dd32:
It uses the same style as current in core(
<form method="get" action="">
)
In current trunk, the search box is inside <form method="post" action="">
, which is the form used for bulk actions. What my patch does is separating the search box into a method="get"
form, which fixes the issue. But still, it works for me, I'm not sure why it doesn't work for you.
#14
@
14 years ago
Sorry about that, I'm here to eat my hat :)
No idea what happened there, but I tested that 3 damn times, and it didn't work.. Just re-applied the patch and it did work..
Either style is fine by then, .3 will probably be best off for 3.2 (due to ajax, and everything in the single form), .4 solves it to 3.0 compat.
#15
@
14 years ago
- Keywords commit added
- Milestone changed from Awaiting Review to 3.1
Bumping to 3.1 for another dev to check
#16
in reply to:
↑ 7
;
follow-up:
↓ 17
@
14 years ago
Replying to garyc40:
solarissmoke's patch will redirect a search executed in
wp-admin/network/plugins.php
towp-admin/plugins.php
. Plus there might be unexpected consequences when performing bulk action (paging being reset to 1 for example).
attachment:16549.5.diff fixes this.
So all of .3, .4 and .5 are working patches. IMHO .3 isn't very future-proof. Between the other two - devs can decide.
#17
in reply to:
↑ 16
@
14 years ago
Replying to solarissmoke:
attachment:16549.5.diff fixes this.
Anything you modify in the method="post"
form affects bulk actions, so some more testing needs to be done with 16549.5.diff.
#18
@
14 years ago
16549.4.diff looks like the way to go, since we don't have sorting involved.
Same after deleting a plugin.