Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#43207 closed defect (bug) (fixed)

`unregister_setting()` does not remove default option filter

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.7
Component: Options, Meta APIs Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

When using register_setting() and providing a 'default' value, that value is set as the default for get_option() calls using a filter. However, when using unregister_setting() for such a setting, the filter hook is not removed again. This seems like an oversight and should be fixed.

For reference, the functionality was introduced in [38910].

Attachments (1)

43207.diff (1.4 KB) - added by flixos90 7 years ago.

Download all attachments as: .zip

Change History (10)

@flixos90
7 years ago

#1 @flixos90
7 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

I just noticed this is not as critical as I had initially thought. Since the setting will not be registered anymore, the filter won't change anything. Regardless, it's redundant that way, so it should be removed again.

43207.diff adds a patch with a test.

#2 @SergeyBiryukov
7 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 5.0

#3 @flixos90
7 years ago

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

#4 @flixos90
7 years ago

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

In 42655:

Options: Unhook default option filter when setting is unregistered.

Fixes #43207.

#5 @johnbillion
7 years ago

In 42659:

Options: Use more appropriate language in the test suite.

See #43207, #38176

#6 follow-up: @jorbin
7 years ago

In 42663:

Options: Use most appropriate language in the test suite.

The test suite's opinion of cancer should be clear.

Reverts r42659.

See #43207, #38176

#7 in reply to: ↑ 6 @helen
7 years ago

Replying to jorbin:

In 42663:

Options: Use most appropriate language in the test suite.

The test suite's opinion of cancer should be clear.

Reverts r42659.

See #43207, #38176

I have discussed this with both involved parties and will be reverting this revert. There will be no further back-and-forth via commits without documented discussion. I will also be reviewing our documentation around communication and ensuring that our socially-understood guideline of family-friendly, non-profane language in open project communication is also a written one. This is indeed a small thing for a lead to pull her lead card on, but here we are.

#8 @helen
7 years ago

In 42667:

Options: Use strings in the test suite that follow community guidelines.

Also somewhat explain to future maintainers why this pairing of strings was chosen, besides this committer's musical preferences. Social opinions on cancer are fairly clear, as are commonly accepted definitions of profanity and family-friendly language.

Shout out to the close contender, which would have been particularly appropriate here: "You could be the king But watch the queen conquer".

see #43207, #38176

Last edited 7 years ago by helen (previous) (diff)

#9 @flixos90
6 years ago

  • Milestone changed from 5.0 to 5.1
Note: See TracTickets for help on using tickets.