Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#34594 closed defect (bug) (fixed)

Possible Bug on Customizer Ordering Method

Reported by: bordoni's profile bordoni Owned by: westonruter's profile 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 10 years ago.
Patch Fixing the possible typo
34594.1.diff (1.9 KB) - added by westonruter 10 years ago.

Download all attachments as: .zip

Change History (8)

@bordoni
10 years ago

Patch Fixing the possible typo

#1 @bordoni
10 years ago

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

#2 @SergeyBiryukov
10 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
10 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
10 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 10 years ago by bordoni (previous) (diff)

@westonruter
10 years ago

#5 @westonruter
10 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
10 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.