[wp-trac] [WordPress Trac] #28637: get_theme_mod returns empty string instead of default

WordPress Trac noreply at wordpress.org
Wed Sep 16 23:19:19 UTC 2015


#28637: get_theme_mod returns empty string instead of default
----------------------------------------+-----------------------------
 Reporter:  philipjohn                  |       Owner:  valendesigns
     Type:  defect (bug)                |      Status:  assigned
 Priority:  normal                      |   Milestone:  4.4
Component:  Customize                   |     Version:  3.9.1
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:  administration
----------------------------------------+-----------------------------

Comment (by valendesigns):

 I've added a patch [attachment:28637.3.diff] that takes into account the
 fact that we more than likely have old data saved and only making a change
 to `set_theme_mod` will not fix the issue at hand. When the expected
 result is the default value but the mod has already saved an empty value,
 changes in `set_theme_mod` will have zero effectiveness until the next
 time the mods are saved. So we need to also add a check in `get_theme_mod`
 for an empty string value, which was discussed earlier but I don't think
 we had a clear view of why it needed to happen. I've added 2 new tests
 that demonstrate that defaults are returned with these simple empty string
 checks.

 I'm certain some serious debate around this patch is required, but it's
 now obvious to me that an empty string is an invalid value. Why would we
 save empty strings to the database, and why would an empty string override
 the default? In what logical situation can we come up with that would make
 an empty string valid? I'm having a difficult time thinking of one. Can
 anyone present a case where an empty string would be the desired result
 and one which `false` could not be used as an alternative?

 The only thing I can think of that is problematic is with user
 assumptions. A situation where the user has explicitly checked for an
 empty string because they found that the function would save this kind of
 value. They're relying on assumptions that should never have existed in
 the first place. Though we do need to account for these kinds of issues it
 shouldn't stop us from fixing incorrect logic and rewriting the
 assumptions of these two functions.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/28637#comment:23>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list