#54597 closed defect (bug) (fixed)
Featured Image section missing on Posts and Pages
Reported by: | noisysocks | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-screenshots has-patch has-unit-tests |
Focuses: | Cc: |
Description
Originally reported in https://github.com/WordPress/twentytwentytwo/issues/273.
Describe the bug
With WP 5.9-beta1 with Twenty Twenty-Two block theme, and when editing a Post (All Posts > Hello World), I don’t see the section to add the Featured Image in the Post Settings sidebar.
If I switch to Twenty Twenty-One, the Featured Image section is there, and if I activate the Gutenberg plugin, the Featured Image section is there in the sidebar.
To Reproduce
Steps to reproduce the behavior:
- Go to 'Posts > All Posts > Hello World (or any other added post)'
- Click on 'Settings > Posts sidebar'
- Scroll down to 'Featured Image section'
- It's not there
Expected behavior
Section to add Featured Image should be in the Set
Screenshots
Attached are screenshots of Twenty Twenty-Two and Twenty Twenty-One
Device:
- Device: Mac Mini
- OS: Mac OS Big Sur 11.6
- Browser: Chrome iVersion 96.0.4664.55 (Official Build) (x86_64) and FireFox 94.0.2 (64bit)
- Version:
Additional context
Attachments (2)
Change History (35)
#2
@
3 years ago
- Keywords has-screenshots added
I can confirm I can reproduce the issue as well on WordPress 5.9 Beta 2 using TT2 theme.
https://i.gyazo.com/e6f8727425f712bcece75e6d352eb62f.mp4
This ticket was mentioned in PR #2029 on WordPress/wordpress-develop by noisysocks.
3 years ago
#3
- Keywords has-patch added; needs-patch removed
By default, block themes should have a few theme supports enabled by default. These are: post-thumbnails, responsive-embeds, editor-styles, html5, automatic-feed-links.
This is a backport of https://github.com/WordPress/gutenberg/pull/35593.
Trac ticket: https://core.trac.wordpress.org/ticket/54597
noisysocks commented on PR #2029:
3 years ago
#4
I tried adding a unit test for this but couldn't quite figure it out. Perhaps it's not possible because setup_theme
is called during bootstrap. Any advice on that is welcome.
#6
@
3 years ago
Test Report
Env
- WordPress 5.9-beta2-52344-src
- Chrome 96.0.4664.93
- Windows 10
- Theme: Twenty Twenty-Two
- Gutenberg Editor
- Plugin: None activated.
Steps to test
- Navigate to
Posts > Add New
. - There should not be a "Featured image" section in the sidebar.
- Apply PR 2029.
- Navigate to
Posts > Add New
. - There should be a "Featured image" section in the sidebar.
- Repeat the process for
Pages > Add New
.
Results
#8
@
3 years ago
- Owner set to audrasjb
- Resolution set to fixed
- Status changed from new to closed
In 52369:
#9
@
3 years ago
Ah, I didn't notice @costdev proposed some unit tests to check for default theme supports.
@hellofromTonya should we open a new ticket to consider adding these before beta 4 or should we just reopen this one and add them before beta 3?
#10
@
3 years ago
Should the list also have 'title-tag'
?
Isn't there a block for logo? (so the list should have 'custom-logo'
?)
I don't know how the blocks are for custom-header or custom-background.
And considering that blocks could be added anywhere, that includes Classic blocks and legacy widgets, so should the list of html5 support include 'gallery'
, 'caption'
, 'search-form'
, 'navigation-widgets'
or would that depend on what the theme provides in its CSS? And if the theme has CSS for those, does it get the proper markup by calling add_theme_support()
itself?
#11
@
3 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening to address the follow-up comments.
#12
@
3 years ago
@costdev Thanks for the tests in 54597-unit-test.diff!
They look good to me, I'm only confused a bit by this line, as there is no such method in the WP_Hook
class:
@covers WP_Hook::should_load_separate_core_block_assets
I guess the intention was to test the should_load_separate_core_block_assets
filter, but in that case, I think we can just mention the function name here:
@covers wp_should_load_separate_core_block_assets
#13
@
3 years ago
@SergeyBiryukov You're right! I'll update the unit tests with:
@covers ::wp_should_load_separate_core_block_assets
I'll wait to submit the updated unit tests until the follow up comments have been addressed, in case this requires more tests.
This ticket was mentioned in Slack in #core-editor by costdev. View the logs.
3 years ago
#15
@
3 years ago
Hello, everyone
Should the list also have 'title-tag'?
I'm not 100% sure about this. @aristath, I think you worked on this part of the code for Block Themes. Do you know if we need to declare support?
Isn't there a block for logo? (so the list should have 'custom-logo'?)
There's a Site Logo block, and it doesn't require declaring support for custom-logo
.
I don't know how the blocks are for custom-header or custom-background.
Site Editor handles these with template parts and Global Styles.
so should the list of html5 support include 'gallery', 'caption', 'search-form', 'navigation-widgets'
The blocks provide their markup; this shouldn't be necessary.
#17
follow-up:
↓ 20
@
3 years ago
so should the list of html5 support include 'gallery', 'caption', 'search-form', 'navigation-widgets'
The blocks provide their markup; this shouldn't be necessary.
You missed the point that the legacy markup can be either html5 or not, dependent on the value of the theme_support
. So for the theme's styles to match the markup, it needs to select which markup to use. Or just go forward with html5 only.
#19
@
3 years ago
- Keywords commit removed
All patches have been committed. Removing the commit
keyword. Leaving the ticket open to resolve the open questions.
#20
in reply to:
↑ 17
@
3 years ago
- Resolution set to fixed
- Status changed from reopened to closed
Replying to Mamaduka:
Should the list also have 'title-tag'?
I'm not 100% sure about this. @aristath, I think you worked on this part of the code for Block Themes. Do you know if we need to declare support?
No the <title>
tag is automatically added in a block theme. Adding add_theme_support( 'title-tag' );
doesn't do anything.
Replying to joyously:
so should the list of html5 support include 'gallery', 'caption', 'search-form', 'navigation-widgets'
The blocks provide their markup; this shouldn't be necessary.
You missed the point that the legacy markup can be either html5 or not, dependent on the value of the
theme_support
. So for the theme's styles to match the markup, it needs to select which markup to use. Or just go forward with html5 only.
Did some testing on 'gallery'
, 'caption'
, 'search-form'
, and 'navigation-widgets'
. Without adding them to the HTML5 theme support array, each renders as HTML5. So by default, they are already HTML5. As @Mamaduka noted, the block has the markup in it.
This makes sense as core blocks should be HTML5 markup IMO given that it was introduced back in 2014. Thinking out loud, supporting legacy and HTML5 in blocks seems unnecessary. That said, if there's a use case, I'd suggest opening an issue in Gutenberg for discussion and discovery as the work would be done there for block packages to be published and then backported to Core.
I'm reclosing this ticket as no additional theme supports are needed for 5.9.
#21
@
3 years ago
No the <title> tag is automatically added in a block theme.
I see that there is now a private function https://developer.wordpress.org/reference/functions/_block_template_render_title_tag/ which does not check the support. That page refers to the old function _wp_render_title_tag()
which did, and that link is 404.
Did some testing on 'gallery', 'caption', 'search-form', and 'navigation-widgets'. Without adding them to the HTML5 theme support array, each renders as HTML5. So by default, they are already HTML5. As @Mamaduka noted, the block has the markup in it.
Again, it sounds like you are talking about blocks, not legacy content which millions of sites still have. The theme_support for html5 refers to legacy functions that generate the search form (widget or direct call), gallery shortcode, caption shortcode, and Menu widgets (this is for a11y).
Or are you saying that using a block theme, the legacy content is somehow transformed? Or that the block theme doesn't have specific styles for anything but blocks so these things don't matter?
@
3 years ago
TT2 theme with using non-block [gallery]
and [caption]
content: Before and after adding gallery
and caption
HTML5 theme supports. No difference.
#22
@
3 years ago
Replying to joyously:
I see that there is now a private function https://developer.wordpress.org/reference/functions/_block_template_render_title_tag/ which does not check the support. That page refers to the old function
_wp_render_title_tag()
which did, and that link is 404.
Right, that private function was added in 5.8.0. In 5.9, in the locate_block_template()
function, the _wp_render_title_tag
callback is unhooked from 'wp_head'
(see https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-template.php#L109). In 5.9, the <title>
tag is automatically rendered for block themes. Non-block themes are not changed or impacted.
Replying to joyously:
Did some testing on 'gallery', 'caption', 'search-form', and 'navigation-widgets'. Without adding them to the HTML5 theme support array, each renders as HTML5. So by default, they are already HTML5. As @Mamaduka noted, the block has the markup in it.
Again, it sounds like you are talking about blocks, not legacy content which millions of sites still have. The theme_support for html5 refers to legacy functions that generate the search form (widget or direct call), gallery shortcode, caption shortcode, and Menu widgets (this is for a11y).
Or are you saying that using a block theme, the legacy content is somehow transformed? Or that the block theme doesn't have specific styles for anything but blocks so these things don't matter?
The default theme supports additions are for block themes. Non-block themes are not impacted.
What about the non-block content when using a block theme? Hmm, I was curious. So I did a test.
- Started with WordPress 4.9 and Twenty Fifteen theme. Added a gallery, image with caption, and search widget.
- Then upgraded to 5.9. Result: No changes in the HTML markup. The original content in the editor shows the
[gallery]
and[caption]
shortcodes. - Then switched to TT2.
- Then added default theme support for
'gallery'
and'caption'
.
See non-block-caption-54597.png. Notice there's no difference in the markup before and after adding HTML5 support for both 'gallery'
and 'caption'
.
What do you think @joyously?
#23
@
3 years ago
That page refers to the old function
_wp_render_title_tag()
which did, and that link is 404.
Yes, but something is wrong with the Code Reference if there is a link to a non-existent page.
Notice there's no difference in the markup before and after adding HTML5 support
https://core.trac.wordpress.org/browser/tags/5.8.1/src/wp-includes/media.php#L2303 is where the support is checked for the gallery shortcode. Without support, a gallery uses dl
, dt
, and dd
with inline style
tag. With it, a gallery uses figure
, div
, and figcaption
and no inline style
tag.
The caption shortcode https://core.trac.wordpress.org/browser/tags/5.8.1/src/wp-includes/media.php#L2202
uses figure
and figcaption
for html5, and div
with p
and an extra 10px of width without html5. So your screenshots show two cases of no html5 support.
#24
follow-up:
↓ 25
@
3 years ago
You're right @joyously. Thanks for reminding me. For some reason, the theme support was not being added correctly in my test. Fixed it. Thanks.
The change in this ticket is targeted at block themes and blocks.
Question: @joyously, for block themes, do you think that Core should automatically default to HTML5 for all content (i.e. block and non-block content)? Or should the block theme itself set the non-block content theme supports (as non-block themes do)?
#25
in reply to:
↑ 24
;
follow-up:
↓ 26
@
3 years ago
Replying to hellofromTonya:
Question: @joyously, for block themes, do you think that Core should automatically default to HTML5 for all content (i.e. block and non-block content)? Or should the block theme itself set the non-block content theme supports (as non-block themes do)?
As far as I can tell, it would make sense if all block themes could only use and provide HTML5 markup. Block themes are a good way to drop non-HTML5 support in long term, if possible :)
#26
in reply to:
↑ 25
@
3 years ago
Replying to audrasjb:
As far as I can tell, it would make sense if all block themes could only use and provide HTML5 markup. Block themes are a good way to drop non-HTML5 support in long term, if possible :)
I agree. Might want to open a new ticket though as the scope of this ticket has grown past the original reported issue. It sounds like we're leaning towards making all block themes HTML5 markup only by default within Core itself.
#27
@
3 years ago
Might want to open a new ticket though as the scope of this ticket has grown past the original reported issue.
It's part of the original patch. Just some parameters got left out.
I think it would be good to force HTML5 for block themes, but that's just an opinion.
As identified in https://github.com/WordPress/twentytwentytwo/issues/273#issuecomment-987023378, the problem is that this Gutenberg PR was missed while updating Core with Gutenberg changes:
https://github.com/WordPress/gutenberg/pull/35593
We need to make the PHP changes in the PR above to Core.