Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#35215 closed defect (bug) (fixed)

Setting help tab priorities fails to correctly order the tabs

Reported by: meitar's profile meitar Owned by: jorbin's profile jorbin
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.6
Component: Help/About Keywords: has-patch has-unit-tests commit fixed-major
Focuses: administration Cc:

Description

When using the new WP_Screen::add_help_tab() feature of priorities, the docs suggest these priorities should be numeric. However, they are not correctly numerically sorted by priority. I had three help tabs with priorities 2, 10, and 40 but WP_Screen::get_help_tabs() returned them in the order 2, 40, 10.

I traced this problem to the way get_help_tabs() was resorting the tabs based on priority, which sorted the tabs by value rather than by their priority keys.

Using ksort() instead of sort() fixes this issue for me. A patch is attached.

Attachments (2)

wp-screen-should-ksort-help-tabs.diff (373 bytes) - added by meitar 8 years ago.
Patch to use ksort() so help tab priorities are returned correctly ordered.
35215.diff (3.2 KB) - added by swissspidy 8 years ago.

Download all attachments as: .zip

Change History (7)

@meitar
8 years ago

Patch to use ksort() so help tab priorities are returned correctly ordered.

#1 @meitar
8 years ago

  • Keywords has-patch added

Seems this bug was introduced when help tab priority was introduced (changeset:34370)?

@swissspidy
8 years ago

#2 @swissspidy
8 years ago

  • Keywords has-unit-tests commit added
  • Milestone changed from Awaiting Review to 4.4.1

Yeah it looks like the priority sorting didn't work properly after [34370].

I just uploaded 35215.diff with updated unit tests that test the behaviour of adding three tabs with priorities of 2, 10 and 40. Without the change to ksort, the tabs are returned in incorrect order.

I also used assertSame in the unit test to make sure the types match all the time.

Moving this to the 4.4.1 milestone since it's a regression.

#3 @jorbin
8 years ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 36089:

Help Tab Order should be based on the Priority Argument

[34370] made the order that tabs are returned respect the order they are added, however it broke the respect of priority. By using a ksort instead of a sort, we can restore that default behavior. This adjusts the unit tests so that both order added and priority are tested.

Props meitar, swissspidy, jorbin
Fixes #35215. See #33941.

#4 @jorbin
8 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

#5 @dd32
8 years ago

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

In 36104:

Help Tab Order should be based on the Priority Argument

[34370] made the order that tabs are returned respect the order they are added, however it broke the respect of priority. By using a ksort instead of a sort, we can restore that default behavior. This adjusts the unit tests so that both order added and priority are tested.

Merges [36089] to the 4.4 branch.
Props meitar, swissspidy, jorbin
Fixes #35215. See #33941.

Note: See TracTickets for help on using tickets.