[3.0] Name the variant a theme actually gave, instead of a blank option - #9446
Open
albertlast wants to merge 1 commit into
Open
[3.0] Name the variant a theme actually gave, instead of a blank option#9446albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
The theme options page builds its variant and colour-mode drop-downs from
Lang::getTxt('variant_' . $variant) and getTxt('colormode_' . $mode), with
?? $variant behind each one to cover a name the language files do not
describe. That fallback has never fired: getTxt() returns '' for a string it
cannot find, never null, so ?? passes the empty string straight through and
the drop-down offers an entry with no label at all.
A theme may name its variants anything it likes, so this is the normal case
for any variant outside the handful the default files know about. With two
variants installed, the list read Default, "", "" - now it reads Default,
light, ocean.
While in here, theme_opt_variant and theme_opt_colormode are asked for from
Themes, but both live in Profile.php, which is where the matching option on
the user's own theme settings page reads them from. The labels do come out
right today, because the theme options page has already loaded Profile.php
by the time these run, but naming a file that does not hold the string makes
Lang::load() reload files to go looking for it.
Signed-off-by: Mathias Albert <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The theme options page (
?action=admin;area=theme;sa=options;th=1) builds its variant and colour-mode drop-downs like this:The
?? $variantis there to cover a variant whose name the language files do not describe. It has never fired.Lang::getTxt()returns''for a string it cannot find — it never returns null — so??passes the empty string straight through and the drop-down offers an option with no label.A theme is free to name its variants anything, so this is the normal case rather than an edge one. With two variants installed, the list reads:
Default,"",""Default,light,oceanSame one-character change for
colormode_*.The second hunk is smaller and not user-visible:
theme_opt_variantandtheme_opt_colormodeare asked for fromThemes, but both strings live inLanguages/en_US/Profile.php— which is where ThemeOptions.php:78 reads them from for the matching option on the user's own settings page. The labels do come out right today because the theme options page has already loadedProfile.phpby the time these run, butLang::loadFileForGetTxt()sees a key that came from an unexpected file and force-reloads to go looking for it. Naming the right file avoids that.Verification
Neither branch is reachable on a stock 3.0 install — the default theme sets
has_dark_mode = falseandtheme_variants = []. Both are reachable for any custom theme that ships variants, and both become reachable here when dark mode lands. To exercise it I temporarily settheme_variants = ['light', 'ocean']andhas_dark_mode = trueinindex.template.phpand read the rendered<option>labels, before and after. That temporary edit is not part of this PR.This is part of the effort to break #7933 into reviewable pieces — found while triaging the admin area, and separated out because it changes no template.
Issues References (Fixes|Related|Closes)
Related: #7933