WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#34594 closed defect (bug) (fixed)

Possible Bug on Customizer Ordering Method

Reported by: bordoni Owned by: westonruter
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.1
Component: Customize Keywords: has-patch commit
Focuses: Cc:

Description

I was looking at the code for Customize Manager class, and found a possible typo on the ordering method; Link to the possible place where the typo might be.

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-customize-manager.php#L1305

Am I right here? I think the correct should be what the patch attached changes.

Attachments (2)

34594.0.patch (562 bytes) - added by bordoni 5 years ago.
Patch Fixing the possible typo
34594.1.diff (1.9 KB) - added by westonruter 5 years ago.

Download all attachments as: .zip

Change History (8)

@bordoni
5 years ago

Patch Fixing the possible typo

#1 @bordoni
5 years ago

  • Component changed from General to Customize
  • Keywords has-patch added

#2 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 4.4
  • Owner set to westonruter
  • Status changed from new to assigned
  • Version changed from trunk to 4.1

Introduced in [30214].

#3 @westonruter
5 years ago

  • Status changed from assigned to reviewing

@bordoni wow, good catch! How did you come across it? Did you notice a defect in the behavior or was it found through studying the codebase?

#4 @bordoni
5 years ago

I was working on something that required a Priority ordering for a The Events Calendar feature, I knew that the Customize would have something like that in place because I've worked with it previously.

So when I was implementing it to the plugin I saw that it was weird, a duplicate $a there, so I checked on trunk and the code was still there since I found it on 4.3.

Never came across the problem on the Customize itself =).

Last edited 5 years ago by bordoni (previous) (diff)

@westonruter
5 years ago

#5 @westonruter
5 years ago

  • Keywords commit added

It turns out that (as far as I can tell) this bug will never be apparent in the Customzier UI because wp.customize.utils.prioritySort did _not_ have this bug, and now that the UI is added via JS, the initial PHP sorting is pretty much irrelevant. It's a good catch regardless.

#6 @westonruter
5 years ago

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

In 35553:

Customize: Fix typo in WP_Customize_Manager::_cmp_priority() which caused unstable sorting for same-priority constructs in PHP.

The issue, however, does not manifest in the UI because the UI is now built via JS and the wp.customize.utils.prioritySort() algorithm did not have the same typo.

Props bordoni, westonruter.
Fixes #34594.

Note: See TracTickets for help on using tickets.