Make WordPress Core

Opened 13 years ago

Closed 10 years ago

#18724 closed defect (bug) (fixed)

Non functioning 'paged' input on plugin search

Reported by: kurtpayne's profile kurtpayne Owned by: ocean90's profile ocean90
Milestone: 4.0.1 Priority: normal
Severity: minor Version: 3.1
Component: Administration Keywords: has-patch fixed-major
Focuses: Cc:

Description (last modified by ocean90)

In the WordPress Admin -> Install Themes -> Search ... The page number input is not functional.

Same for plugin search.

Attachments (8)

non_functioning_input.png (202.3 KB) - added by kurtpayne 13 years ago.
Non functioning paged input
18724.patch (5.8 KB) - added by ocean90 13 years ago.
18724_jk.patch (6.5 KB) - added by johnkleinschmidt 13 years ago.
Patch that doesn't break search
18724_kt.patch (654 bytes) - added by karpstrucking 10 years ago.
18724.diff (2.0 KB) - added by jesin 10 years ago.
18724_kt.2.patch (682 bytes) - added by jesin 10 years ago.
Patch 18724_kt.patch with esc_attr() for $tab
18724.2.patch (1.8 KB) - added by ocean90 10 years ago.
29829.diff (2.3 KB) - added by jorbin 10 years ago.

Download all attachments as: .zip

Change History (40)

@kurtpayne
13 years ago

Non functioning paged input

#1 @ocean90
13 years ago

  • Description modified (diff)
  • Summary changed from Non functioning 'paged' input on theme search to Non functioning 'paged' input on theme/plugin search

Same for plugin search.

#2 @ocean90
13 years ago

  • Version changed from 3.2.1 to 3.1

#3 @ocean90
13 years ago

  • Keywords has-patch added; needs-patch removed

The problem is that the input fields aren't in a <form> tag.

@ocean90
13 years ago

#4 @kurtpayne
13 years ago

@ocean90, thank you. I tested this patch with trunk. It works for the themes and plugins search results.

#5 @kawauso
13 years ago

  • Cc kawauso added

#6 @ocean90
13 years ago

  • Milestone changed from Awaiting Review to 3.3

#7 @DrewAPicture
13 years ago

Closed #19128 as duplicate.

Tested your patch at revision 19130 and Theme Search no longer works so I can't test the pagination. Reverted the patch, search works and pagination doesn't.

If it matters, I'm using latest Chrome, Firefox and Safari, haven't tested others. Plugin search appears to work just fine.

@johnkleinschmidt
13 years ago

Patch that doesn't break search

#8 follow-up: @johnkleinschmidt
13 years ago

  • Cc johnkleinschmidt added
  • Keywords needs-testing added

The issue with ocean90's patch was that he removed/moved the form declarations from the install_theme_search_form function in wp-admin/includes/theme-install.php and the install_search_form function in wp-admin/includes/plugin-install.php. The reason this is a problem is that those functions actually get called two places:

For themes:

  • install_themes_dashboard function in wp-admin/includes/theme-install.php
  • fired from the install_themes_table_header action in wp-admin/includes/class-wp-theme-install-list-table.php

For plugins:

  • install_dashboard function in wp-admin/includes/plugin-install.php
  • fired from the install_plugins_table_header action in wp-admin/includes/class-wp-plugin-install-list-table.php

ocean90's solution was to move the form declarations from the search_form functions to the code that fires the actions, but that means that when the dashboard functions get called they don't have the form declaration.

My patch takes a different approach. It seems to me to make more sense to leave the form declarations in the search_form functions and then to move the call to generate pagination within the search_form function. Its a cleaner approach that avoids having to declare the form DOM elements twice.

#9 in reply to: ↑ 8 ; follow-up: @DrewAPicture
13 years ago

Replying to johnkleinschmidt:

My patch takes a different approach. It seems to me to make more sense to leave the form declarations in the search_form functions and then to move the call to generate pagination within the search_form function. Its a cleaner approach that avoids having to declare the form DOM elements twice.

I haven't tried your patch yet, but did/could you also incorporate Jane's request in #19123 for duplicating the entered pagination to the bottom of the screen as well?

#10 in reply to: ↑ 9 @johnkleinschmidt
13 years ago

Replying to DrewAPicture:

I haven't tried your patch yet, but did/could you also incorporate Jane's request in #19123 for duplicating the entered pagination to the bottom of the screen as well?

I did not change the bottom navigation. I thought that there was a feature freeze for 3.3 and 19123 is listed as an enhancement.

#11 @ryan
13 years ago

  • Milestone changed from 3.3 to Future Release

Per bug scrub, we're punting the list table search/paging bugs to 3.4, where they will be dealt with in a hopefully consistent manner.

#13 @ocean90
12 years ago

#19123 was marked as a duplicate.

#14 @karpstrucking
10 years ago

  • Summary changed from Non functioning 'paged' input on theme/plugin search to Non functioning 'paged' input on plugin search

This seems to no longer affect the theme page thanks to infinite scroll. Perhaps I'm over-simplifying this patch?

#15 @karpstrucking
10 years ago

related/duplicate #29481

#16 @jesin
10 years ago

#29481 was marked as a duplicate.

@jesin
10 years ago

#17 @jesin
10 years ago

Patch 18724.diff handles "paged" input similar to the wp-admin/edit-tags.php and wp-admin/edit.php pages.

Patch 18724_kt.patch which is much more simpler also works as desired.

#18 @nacin
10 years ago

  • Milestone changed from Future Release to 4.0.1

I'm tempted to fix this in 4.0.1.

#19 @nacin
10 years ago

Does 18724_kt.patch have any side effects? Or does this look good to everyone? Note: I'd like an esc_attr() added there, even though $tab is safe.

@jesin
10 years ago

Patch 18724_kt.patch with esc_attr() for $tab

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

#21 @nacin
10 years ago

  • Owner set to ocean90
  • Status changed from new to assigned

@ocean90
10 years ago

#22 @ocean90
10 years ago

18724_kt.2.patch doesn't work for searches. 18724.diff looks better in this case.

18724.2.patch is based on 18724.diff but removes the useless nonce and checks for $pagenum <= $total_pages.

This ticket was mentioned in IRC in #wordpress-dev by nacin1. View the logs.


10 years ago

#24 @aaires
10 years ago

Tested with "4.1-alpha-src" as of today and it works for me.

#25 @mariusvetrici
10 years ago

The patch works fine in the plugin search with a small observation noted as a separate item here:
https://core.trac.wordpress.org/ticket/29791

In the theme's search, because of infinite scrolling, the bug does not apply anymore.

#26 @pandulu
10 years ago

Patch works for me too, tested on WP 4.0.

I can also confirm the observation from @mariusvetrici with the keyword "press" (https://core.trac.wordpress.org/ticket/29791), but had also another observation.

  • A plugin search for "media" resulted in 88 Pages.
  • When i enter 88 and hit return i get to page 88, plugins show up, but the total jumps to "of 101" pages.
  • Also, the number of found items is going up from 2'615 to 3'008 total items.

#27 @aaires
10 years ago

Upon a further look there is a side effect with the patch, reported by ticket #29796. Currently looking into this.

#28 @ocean90
10 years ago

In 29829:

Plugin search: Wrap results in a form to fix pagination's paged input field.

props jesin, ocean90.
see #18724, for trunk.

This ticket was mentioned in Slack in #core by joostdevalk. View the logs.


10 years ago

#30 @ocean90
10 years ago

  • Keywords fixed-major added; needs-testing removed

@jorbin
10 years ago

#31 @jorbin
10 years ago

The above patch is based off the 4.0 branch and is an exact replica of r29829.

I've tested and confirmed that it fixes the issue and that #29796 is not reopened by it.

It is ready to be merged.

#32 @nacin
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 30249:

Plugin search: Wrap results in a form to fix pagination's paged input field.

Merges [29829] to the 4.0 branch.

props jesin, ocean90.
fixes #18724.

Note: See TracTickets for help on using tickets.