WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 5 months ago

#18724 closed defect (bug) (fixed)

Non functioning 'paged' input on plugin search

Reported by: kurtpayne Owned by: 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 4 years ago.
Non functioning paged input
18724.patch (5.8 KB) - added by ocean90 4 years ago.
18724_jk.patch (6.5 KB) - added by johnkleinschmidt 3 years ago.
Patch that doesn't break search
18724_kt.patch (654 bytes) - added by karpstrucking 8 months ago.
18724.diff (2.0 KB) - added by jesin 8 months ago.
18724_kt.2.patch (682 bytes) - added by jesin 7 months ago.
Patch 18724_kt.patch with esc_attr() for $tab
18724.2.patch (1.8 KB) - added by ocean90 7 months ago.
29829.diff (2.3 KB) - added by jorbin 5 months ago.

Download all attachments as: .zip

Change History (40)

@kurtpayne4 years ago

Non functioning paged input

comment:1 @ocean904 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.

comment:2 @ocean904 years ago

  • Version changed from 3.2.1 to 3.1

comment:3 @ocean904 years ago

  • Keywords has-patch added; needs-patch removed

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

@ocean904 years ago

comment:4 @kurtpayne4 years ago

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

comment:5 @kawauso4 years ago

  • Cc kawauso added

comment:6 @ocean904 years ago

  • Milestone changed from Awaiting Review to 3.3

comment:7 @DrewAPicture3 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.

@johnkleinschmidt3 years ago

Patch that doesn't break search

comment:8 follow-up: @johnkleinschmidt3 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.

comment:9 in reply to: ↑ 8 ; follow-up: @DrewAPicture3 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?

comment:10 in reply to: ↑ 9 @johnkleinschmidt3 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.

comment:11 @ryan3 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.

comment:13 @ocean9022 months ago

#19123 was marked as a duplicate.

@karpstrucking8 months ago

comment:14 @karpstrucking8 months 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?

comment:15 @karpstrucking8 months ago

related/duplicate #29481

comment:16 @jesin8 months ago

#29481 was marked as a duplicate.

@jesin8 months ago

comment:17 @jesin8 months 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.

comment:18 @nacin7 months ago

  • Milestone changed from Future Release to 4.0.1

I'm tempted to fix this in 4.0.1.

comment:19 @nacin7 months 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.

@jesin7 months ago

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

comment:20 @ircbot7 months ago

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

comment:21 @nacin7 months ago

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

@ocean907 months ago

comment:22 @ocean907 months 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.

comment:23 @ircbot7 months ago

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

comment:24 @aaires7 months ago

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

comment:25 @mariusvetrici7 months 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.

comment:26 @pandulu7 months 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.

comment:27 @aaires7 months ago

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

comment:28 @ocean907 months ago

In 29829:

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

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

comment:29 @slackbot5 months ago

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

comment:30 @ocean905 months ago

  • Keywords fixed-major added; needs-testing removed

@jorbin5 months ago

comment:31 @jorbin5 months 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.

comment:32 @nacin5 months 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.