Opened 10 years ago
Last modified 2 months ago
#33600 new enhancement
Add `theme_mods_{$stylesheet}` option during `populate_options()`
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Themes | Keywords: | has-patch has-unit-tests needs-test-info |
Focuses: | Cc: |
Description
After switch_theme()
, a theme_mods_{$stylesheet}
option is created if one doesn't exist to avoid extra database queries (if I understand #14828 correctly).
No similar option is currently created during installation, so a fresh install can includes those queries if the default theme looks for the mods before the option is added.
The attached patch would add the option during populate_options()
.
I included a unit test, although I'm not sure where the best location for it would be.
Attachments (1)
Change History (11)
#1
@
8 years ago
- Keywords has-patch has-unit-tests needs-testing added
- Milestone changed from Awaiting Review to Future Release
This ticket was mentioned in Slack in #themereview by joyously. View the logs.
7 years ago
This ticket was mentioned in PR #8525 on WordPress/wordpress-develop by @SirLouen.
4 months ago
#4
Adapted a 10-year-old ticket by @dlh
Trac ticket: https://core.trac.wordpress.org/ticket/33600
#5
@
4 months ago
- Keywords needs-testing-info added; needs-testing removed
@dlh patch was not applying so I adapted it.
The unit test is passing correctly after introducing the mods, but still, I cannot figure out the purpose of all this.
Could you provide some testing instructions to reproduce the potential issue with and without unit-tests?
#6
follow-up:
↓ 7
@
4 months ago
The problem occurs when get_theme_mod()
is called before set_theme_mod()
is, if it ever is. The behavior was described in more detail in #14828, and it was fixed there for theme activations, but not new installations.
The wp_get_custom_css_post()
function, introduced in 4.7, saves a theme mod, and since that function is called regularly, the change proposed in this ticket has little practical impact today, although I still think it's technically sound.
#7
in reply to:
↑ 6
@
4 months ago
- Keywords close added; has-patch has-unit-tests needs-testing-info removed
Replying to dlh:
The problem occurs when
get_theme_mod()
is called beforeset_theme_mod()
is, if it ever is.
This is one topic I'm commenting lately in the core test group. Many patches are coming too theoretical without a real use case. And are sometimes so entangled, that as you say "if it ever is", can be, technically never or even, as I've seen in some patches, in a wrong coding scenario (not sure which is this case).
the change proposed in this ticket has little practical impact today, although I still think it's technically sound.
So then we can say we can confirm to close this very old ticket, because it's not necessary anymore. If someone happens to locate an issue in the future, can just re-open the ticket.
#8
follow-up:
↓ 9
@
4 months ago
- Keywords has-patch has-unit-tests added
I have sympathy for your point, but I think #14828 demonstrated that the behavior in question is a real bug, not just a theory or the result of developer error.
Anyway, I'll leave the close
keyword and let a committer or someone else decide what to do, but I restored the other keywords, because the ticket still has a patch and tests :)
#9
in reply to:
↑ 8
@
4 months ago
- Keywords needs-testing-info added; close removed
Replying to dlh:
I have sympathy for your point, but I think #14828 demonstrated that the behavior in question is a real bug, not just a theory or the result of developer error.
Anyway, I'll leave the
close
keyword and let a committer or someone else decide what to do, but I restored the other keywords, because the ticket still has a patch and tests :)
@dlh makes sense, if this can happen we should get it fixed. I can't visualize any scenario where this can trigger the bug
Could provide a use-case
with a micro plugin or simply some code snippet, where we can test the error being triggered?
PS: Btw I don't understand why but the report is marked as enhancement
, not as bug
Hmm, my theme actually uses the fact that it is not populated to supply default values and know that it is brand new.