Opened 5 years ago
Last modified 4 months ago
#48734 accepted defect (bug)
Twenty Twenty: [em] tag with blank string inside ruins theme layout
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 5.3 |
Component: | Bundled Theme | Keywords: | has-screenshots has-patch needs-testing 2nd-opinion |
Focuses: | Cc: |
Description
Hello!
I’ve found a layout issue, concerning to "em" tag.
When we use the "em" tag and try to separate paragraphs inside this tag with blank strings, all the parts that are not in “touch” with opening and closing tags are "on their own" in layout. Tags were used in Classic Editor. See the screenshot below.
Here’s the source:
https://drive.google.com/file/d/1KWMwDFlodidBTqjDJRfP9j0mmw6QMuo8/view?usp=sharing
And here’s the result:
https://drive.google.com/file/d/1Tzhi-hsaGDLDdJax8kMUonHe7tJaX2Q7/view?usp=sharing
Here's addition example in English. The source code:
<em>This is some random text on the first line Here is more text on a second line Additional text on a 3rd line with the closing EM tag</em>
And here's the result
https://drive.google.com/file/d/1v9R127wdjPHjlXThMe9zjKXWz6V3xk8Y/view?usp=sharing
Tested on clean Twenty Twenty theme without any additionsl CSS code.
Tested on two different sites.
The issue is recreated for WordPress versions 4.7.15, 5.2.4 and 5.3.
One more experiment: if I put the strings in <p>...</p> tag, all the strings go to the left and ignore theme styles.
Also tested the case for Twenty Nineteen: there is no such issue.
Attachments (4)
Change History (21)
#1
@
5 years ago
- Focuses css template removed
- Keywords needs-patch has-screenshots added
- Milestone changed from Awaiting Review to 5.3.1
- Owner set to audrasjb
- Status changed from new to accepted
Hi there, thank you for this ticket @derlynad and welcome to WordPress Trac!
I can reproduce the issue on my side, though it's not related to <em> tag.
See video screenshot above.
Investigating…
#2
follow-up:
↓ 6
@
5 years ago
The issue doesn't appears anymore if I remove this bit of code from functions.php
:
/** * Output Customizer settings in the classic editor. * Adds styles to the head of the TinyMCE iframe. Kudos to @Otto42 for the original solution. * * @param array $mce_init TinyMCE styles. * * @return array $mce_init TinyMCE styles. */ function twentytwenty_add_classic_editor_customizer_styles( $mce_init ) { $styles = twentytwenty_get_customizer_css( 'classic-editor' ); if ( ! isset( $mce_init['content_style'] ) ) { $mce_init['content_style'] = $styles . ' '; } else { $mce_init['content_style'] .= ' ' . $styles . ' '; } return $mce_init; }
#4
follow-up:
↓ 5
@
5 years ago
- Keywords reporter-feedback added; needs-patch removed
It looks like the issue only occurs when editing the Hello World default post.
Other posts are looking good on my side.
Can you reproduce that @derlynad ?
#5
in reply to:
↑ 4
@
5 years ago
Replying to audrasjb:
It looks like the issue only occurs when editing the Hello World default post.
Other posts looks good on my side.
Can you reproduce that @derlynad ?
@audrasjb I reproduced it on a sample page in WordPress 5.2.4 (new site) but the first time I noticed this issue on the old posts on my blog (WordPress 4.7.15), so it wasn't connected to default pages.
Created new page in WordPress 4.7.15 and checked one more time. The issue is still reproduceable.
#6
in reply to:
↑ 2
@
5 years ago
Replying to audrasjb:
The issue doesn't appears anymore if I remove this bit of code from
functions.php
:
/** * Output Customizer settings in the classic editor. * Adds styles to the head of the TinyMCE iframe. Kudos to @Otto42 for the original solution. * * @param array $mce_init TinyMCE styles. * * @return array $mce_init TinyMCE styles. */ function twentytwenty_add_classic_editor_customizer_styles( $mce_init ) { $styles = twentytwenty_get_customizer_css( 'classic-editor' ); if ( ! isset( $mce_init['content_style'] ) ) { $mce_init['content_style'] = $styles . ' '; } else { $mce_init['content_style'] .= ' ' . $styles . ' '; } return $mce_init; }
Tried to remove this fragment of code, but the issue is still here.
#7
follow-up:
↓ 8
@
5 years ago
@derlynad yes, removing those lines don't do anything, I messed up with my tests, sorry.
I can reproduce the issue on every default theme, like Twenty Nineteen for example. It seems to happen only when editing posts made with or for the Block Editor. Like default posts/pages or posts/pages created with the default editor. It doesn't seems to happen with new posts/pages manually created with the Classic Editor.
Can you reproduce that on your side?
#8
in reply to:
↑ 7
@
5 years ago
Replying to audrasjb:
@derlynad yes, removing those lines don't do anything, I messed up with my tests, sorry.
I can reproduce the issue on every default theme, like Twenty Nineteen for example. It seems to happen only when editing posts made with or for the Block Editor. Like default posts/pages or posts/pages created with the default editor. It doesn't seems to happen with new posts/pages manually created with the Classic Editor.
Can you reproduce that on your side?
Yes, I can. I've tested it today, created new page in WP 5.3 in block editor, classic block, html mode. Here's what's happening when we enter preview mode in Twenty Twenty:
https://drive.google.com/file/d/1X1q8yPGwN4-ZBZr_laPCLwo8p9GqlnK7/view
Here's the tag transformation in the block after saving the page and switching to visual editing mode:
https://drive.google.com/file/d/1wL61TWbMIrJgO34dd_HkUIaDCLg58N5v/view
Somehow em tag was automatically closed before empty string.
Here's how it looks like on Twenty Nineteen preview:
https://drive.google.com/file/d/1Lbalf2_9n-nOisrX-DL0DXBbUlOCD1s8/view
Looks like everything is OK.
Oh, and by the way. If we change EM tag with STRONG one, the issue is the same, here's the screenshot:
https://drive.google.com/file/d/1Q3nQKnIk6HcrEx3MIDcnKkOkX_uSKIRK/view
Hope it helps.
#11
@
5 years ago
- Keywords has-patch needs-testing added
The Classic Editor and the Classic block try to make HTML valid, which includes efforts to close tags.
Using Twenty Twenty and the Classic Editor, I added four paragraphs within an <em>
tag. The em
tag started at the beginning of the first line and closed at the end of the fourth paragraph's line. Each paragraph was separated by an empty line (instead of explicitly adding the <p>
tags).
The editor correctly adjusted the first and last paragraphs by putting the em
tags inside the paragraph tags. But the second and third paragraphs were placed inside an em
tag. That causes a misalignment in Twenty Twenty because the em
does not currently honor the width
and max-width
intended for paragraph and heading tags.
I see two different ways of fixing this for similar situations:
- Make sure the editor properly recognizes that all of those paragraphs should nest any
em
orstrong
tags inside each paragraph tag. - Edit the stylesheet for Twenty Twenty so that any
em
orstrong
tags (but notspan
) immediately within the.entry-content
section can honor the width styles:.entry-content > strong, .entry-content > em { display: block; }
(I'll upload a patch for the second option.)
However, for better semantics and readability in this case, I highly recommend not having so much text in an emphasis tag and/or italicized (see ticket:47327 for some reasons).
Or if all of that text should be italicized, it could be better inside a div
tag. Note that this option requires the empty line after the opening tag in Classic Editor, and it might combine paragraphs if you switch from Text to the Visual editor.
<div style="font-style: italic;"> paragraph 1 paragraph 2 paragraph 3 paragraph 4 </div>
#13
@
8 months ago
@audrasjb I think this is assigned to you but not being progressed right now so I am going to remove you so it can continue on it's journey. If that isn't correct we can always add you back in.
#15
@
8 months ago
Thank you @shailu25. Before I do progress though, I am just checking here but it seems a few things on the refresh and I want to be sure before I move to commit.
- It depends on the two mentioned tickets [47820] and [47846].
- Once those are done this should then be committed and only then.
@sabernhardt is that your expectation here so I can make sure to be aligned.
#16
@
8 months ago
- Keywords 2nd-opinion added
Thanks for updating the patch. I'm not so sure about setting display: block
now.
Having a strong
or em
element without a parent div
or paragraph is already an edge case, and the situation described on this ticket is actually caused by invalid HTML.
The case of someone actually wanting parent-less inline elements to continue displaying inline is likely even less common, but it technically is possible. Adding text <em>without</em> paragraph
in a Custom HTML block did not honor the margin, and display: block
made it more broken than it already was.
Also, the numbers I mentioned earlier are the changesets that made the first patch unusable, not ticket numbers.
Twenty Twenty VS Twenty Nineteen - Classic Editor Text Mode