#54645 closed defect (bug) (fixed)
Allow numeric theme name
Reported by: | alvastar | Owned by: | spacedmonkey |
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | 5.7 |
Component: | Themes | Keywords: | has-patch has-unit-tests has-testing-info |
Focuses: | administration, template, multisite | Cc: |
Description
What steps should be taken to consistently reproduce the problem?
- Create a theme with a numeric name or rename an existing one.
- Activate the theme for your site.
The theme will not be displayed as active.
Also, when using the Gutenberg editor, calls to the add_theme_support function will not work.
Possibly the problem is how PHP is converting the keys of the array:
Strings containing valid decimal ints, unless the number is preceded by a + sign, will be cast to the int type. E.g. the key "8" will actually be stored under 8. On the other hand "08" will not be cast, as it isn't a valid decimal integer.
Suggested solution to the problem:
File in wp-icnludes/rest-api/endpoints/class-wp-rest-themes-controller.php replace this
/**
* Helper function to compare two themes.
*
* @since 5.7.0
*
* @param WP_Theme $theme_a First theme to compare.
* @param WP_Theme $theme_b Second theme to compare.
* @return bool
*/
protected function is_same_theme( $theme_a, $theme_b ) {
return $theme_a->get_stylesheet() === $theme_b->get_stylesheet();
}
with this
/**
* Helper function to compare two themes.
*
* @since 5.7.0
*
* @param WP_Theme $theme_a First theme to compare.
* @param WP_Theme $theme_b Second theme to compare.
* @return bool
*/
protected function is_same_theme( $theme_a, $theme_b ) {
return (string)$theme_a->get_stylesheet() === (string)$theme_b->get_stylesheet();
}
Attachments (2)
Change History (23)
#1
@
3 years ago
- Keywords needs-patch needs-unit-tests added
- Milestone changed from Awaiting Review to Future Release
- Owner set to spacedmonkey
- Status changed from new to assigned
- Version changed from 5.8.2 to 5.7
This ticket was mentioned in PR #2758 on WordPress/wordpress-develop by enricobattocchi.
3 years ago
#3
- Keywords has-patch added; needs-patch removed
Casts theme dir name to string when creating a WP_Theme object, allowing to have numerical dirnames for themes.
Trac ticket: https://core.trac.wordpress.org/ticket/54645
#4
follow-up:
↓ 6
@
3 years ago
- Keywords needs-patch needs-testing added; needs-unit-tests has-patch removed
To clarify: having a numerical name (in style.css
) doesn't seem to create problems, while having a numerical folder name does.
The patch fixes this by making sure that we cast the theme dirname to a string when creating the when assigning to the WP_Theme
objectstylesheet
property in the WP_Theme
contructor.
To test, rename e.g. wp-content/themes/twentytwentyone
to wp-content/themes/2021
.
Without the patch:
- setting the theme as "Active" doesn't result in being displayed as such in
Appearance
>Themes
- editing a post in the block editor shows e.g. no section to set featured image in the sidebar
With this patch:
- setting the theme as "Active" results in being displayed as such in
Appearance
>Themes
- editing a post in the block editor shows the supported features e.g. the featured image
I'm removing the Unit test added after change of approach.needs-unit-tests
keyword since there's currently no default theme with a numerical dirname to be retrieved by a call to wp_get_themes()
#6
in reply to:
↑ 4
;
follow-up:
↓ 7
@
3 years ago
- Keywords needs-unit-tests added
Replying to lopo:
I'm removing the
needs-unit-tests
keyword since there's currently no default theme with a numerical dirname to be retrieved by a call towp_get_themes()
.
Thanks for the patch! I think this change would still benefit from unit tests to make sure the fix works as expected and there are no regressions at a later point.
#7
in reply to:
↑ 6
@
3 years ago
Replying to SergeyBiryukov:
Replying to lopo:
I'm removing the
needs-unit-tests
keyword since there's currently no default theme with a numerical dirname to be retrieved by a call towp_get_themes()
.
Thanks for the patch! I think this change would still benefit from unit tests to make sure the fix works as expected and there are no regressions at a later point.
Would love to, @SergeyBiryukov, but I'm not sure how it can be done without adding a theme to the default ones?
I changed the approach by moving the cast inside the WP_Theme
constructor so I could add a unit test for that.
#11
@
2 years ago
Test Report
This report validates that the indicated patch addresses the issue.
Patch tested: REPLACE_WITH_PATCH_URL
Environment
- OS: macOS 12.3.1
- Web Server: Nginx
- PHP: 7.4.29
- WordPress: 6.1-alpha-53344-src
- Browser: Chrome 105.0.5195.102 (Official Build) (x86_64)
- Theme: Twenty Twenty-Two (and Twenty Twenty-One)
- Active Plugins:
Actual Results
- ✅ Issue resolved with patch.
- All worked as expected
Additional Notes
- Any additional details worth mention.
- Also tested with other interesting characters such as
$$$
for the filename or 2021.2021 and worked as expected.
#12
@
2 years ago
- Keywords needs-testing removed
Hello All, I also got a chance to test this issue with some other theme like, Numeric, Alphanumeric, etc and PR https://github.com/WordPress/wordpress-develop/pull/2758 fixed the issue.
Test Report
Env
- WordPress - 6.1-beta1-54282-src
- Chrome Version - 105.0.5195.125 (Official Build) (x86_64)
- OS - macOS Monterey V12.3.1
- Theme: Twenty Twenty Two
- PHP - 8.0.0
- Web Server - Apache
- Database - MySQL 5.7.28
- Used PR - https://github.com/WordPress/wordpress-develop/pull/2758
Steps to test
- Rename the theme name from style.css and folder name like 'wp-content/themes/2022' or 'wp-content/themes/AB123'
- Go to Appearance > Themes
- Activate your theme which has a name set in step-1
Test result
A theme that has names like 'ABC2022' / '2022' / 'Twenty Twenty Two', etc is appearing as active (if activated) with available option 'Customize'.
Screencast example:
Before Fix -
After Fix by PR2758 -
#13
@
2 years ago
- Milestone changed from 6.1 to 6.2
Sadly didn't get to this ticket in 6.1 release. Punting to 6.2
#14
@
2 years ago
Test Report
This report validates that the indicated patch addresses the issue.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/2758
Environment
- OS: macOS 13.0
- Web Server: Nginx
- PHP: 7.4.30
- WordPress: 6.2-alpha-54642-src
- Browser: Google Chrome 108.0.5359.124, Mozilla Firefox 108.0.1(64bit)
- Theme: Twenty Ten, Twenty Twenty-Three
Actual Results
- ✅Issue resolved with patch.
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
#16
@
22 months ago
- Keywords has-testing-info added
@alvastar thanks for the ticket!
This ticket was discussed during the recent bug scrub. It looks like it's unlikely that work will be done on this during the 6.2 cycle.
According to the manual test report, the ticket is operational.
@spacedmonkey Is this still possible to land in 6.2, or should it be moved to Future Release for now?
This ticket was mentioned in Slack in #core by costdev. View the logs.
22 months ago
@SergeyBiryukov commented on PR #2758:
21 months ago
#21
Thanks for the PR! Merged in r55426.
#54666 was marked as a duplicate.