WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 2 months ago

#48617 assigned defect (bug)

Twenty Twenty: Figure elements with inline style can overflow content bounds

Reported by: Anlino Owned by: nielslange
Milestone: Future Release Priority: normal
Severity: normal Version: 5.3
Component: Bundled Theme Keywords: needs-testing has-patch
Focuses: css Cc:

Description

In content created in the classic editor, images that have captions and are set to alignnone or aligncenter can exceed the width of the entry content. This is caused by the inline style on the figure element overwriting the width set on .entry-content > * in style.css.

Steps to reproduce:

  1. Add an image while using the classic editor.
  2. Set it to alignnone or aligncenter.
  3. Add a caption.
  4. Check on a screen size thinner than the width of the image.

Suggested solution:
(Condensed for clarity)

.entry-content figure.alignnone[style*="width"],
.entry-content figure.aligncenter[style*="width"] {
        max-width: calc(100% - 4rem) !important;
}

@media ( min-width: 620px ) {
        body:not(.template-full-width) .entry-content figure.alignnone[style*="width"],
        body:not(.template-full-width) .entry-content figure.aligncenter[style*="width"] {
                max-width: 58rem !important;
        }
}

@media ( min-width: 1280px ) {
        body.template-full-width .entry-content figure.alignnone[style*="width"],
        body.template-full-width .entry-content figure.aligncenter[style*="width"] {
                max-width: 120rem !important;
        }
}

First reported by @derlynad.

Attachments (3)

48617.diff (1.7 KB) - added by samful 2 months ago.
48617.2.diff (1.8 KB) - added by samful 2 months ago.
48617.3.diff (1.8 KB) - added by samful 3 weeks ago.
refreshed with proper /src path and line numbers

Download all attachments as: .zip

Change History (16)

#1 @SergeyBiryukov
8 months ago

  • Milestone changed from Awaiting Review to 5.3.1

#2 @nielslange
8 months ago

  • Owner set to nielslange
  • Status changed from new to assigned

#3 @nielslange
8 months ago

Happy to create a patch for this issue later today, @Anlino.

#4 @nielslange
8 months ago

  • Keywords reporter-feedback added

@Anlino I'm unable to reproduce this issue using an large image and the following code:

[caption id="attachment_1818" align="aligncenter" width="2560"]<img class="wp-image-1818 size-full" src="http://core.trac.test/wp-content/uploads/2019/11/evgeni-tcherkasski-SHA85I0G8K4-unsplash-scaled.jpg" alt="Lighthouse in the night" width="2560" height="1708" /> Lighthouse in the night[/caption]

Could you share your HTML code of this issue?

#5 @Anlino
8 months ago

@nielslange Sorry for the late reply. Using the code you posted causes the issue for me in the Classic Editor and the Classic block in the Block Editor. Screen capture here: https://www.andersnoren.se/private/media/trac/wp-caption-overflow.png

Note the image spanning the entire width of the screen, and the width of the figure being 580px even though the screen is only 375px wide.

#6 @ianbelanger
8 months ago

  • Milestone changed from 5.3.1 to Future Release

#7 @ianbelanger
6 months ago

  • Keywords needs-patch needs-testing added; reporter-feedback removed
  • Milestone changed from Future Release to 5.4

I would like to get this into the next major release, however I would prefer that we avoid using !important if possible. I will test and look for a solution when I get a chance, but any help with this issue is appreciated.

#8 @ianbelanger
5 months ago

  • Milestone changed from 5.4 to Future Release

This is not going to be ready for 5.4 Beta 1 which is today, so punting to a Future Release.

#9 @samful
2 months ago

I wanted to help out @ianbelanger. To remove the !important I had to also exclude the figure elements we are working on using :not, from the rule that was overriding the rules used in the Suggested solution. (this was the entry-content > *:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.is-style-wide) rule.)

Then I simply put the condensed Suggested solution CSS into the style.css file in the correct places without the !important. Please check my patch file, I expect the style-rtl.css file will need to be changed in the same way if approved.

@samful
2 months ago

#10 @samful
2 months ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core by howdy_mcgee. View the logs.


2 months ago

#12 @ianbelanger
2 months ago

  • Focuses css added
  • Keywords needs-refresh added

Thanks for submitting the patch @samful, unfortunately in my testing, the patch does not fix the issue.

First, your patch would not apply for me, I had a line endings error when trying to apply it. So I took your CSS and added to style.css in my local install. It did add the margins to images, with captions that were set to alignnone or aligncenter and added using the classic editor, but it also removed the margins from all other items on the page. Paragraphs, images added as blocks, etc...

I am not exactly sure why this happened yet, but I am investigating further.

#13 @samful
2 months ago

  • Keywords needs-refresh removed

Apologies for that @ianbelanger , I failed in testing this properly and will take more care in the future... The issue seemed to be because of the :not CSS seemingly not supporting more specific targeting like :not(figure.aligncenter) , I can't seem to find example of this online, so I'm assuming this isn't supported and I changed it to a more broad :not(.aligncenter).

I tested this patch on the sample page using blocks and added a few paragraphs of text under my image in the classic editor and the margins seem to be back now. I just hope my not rule isn't too broad now.

@samful
2 months ago

@samful
3 weeks ago

refreshed with proper /src path and line numbers

Note: See TracTickets for help on using tickets.