Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#55393 closed defect (bug) (fixed)

File name URL overflow the container in the media editor

Reported by: mitogh's profile mitogh Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 5.4
Component: Media Keywords: good-first-bug has-patch has-screenshots commit
Focuses: ui, css, administration Cc:

Description

When the filename is large the container where the link of the filename is presented overflow the container in the Edit Media area.

Steps to replicate.

  1. Upload the attached image to the media library
  2. Click on Edit to open the edit media area.
  3. Observe the result.

Dimensions where the issue is happening: 16843x1417

Attachments (16)

2022-03-15_13-03.png (836.3 KB) - added by mitogh 2 years ago.
unsplash-image-Zw3n0QGXMxs6230e225cd4620.78824906.jpeg (2.9 MB) - added by mitogh 2 years ago.
77EA961B-74BC-4FDF-8ABD-0DDFE4CECC7C.jpg (2.3 MB) - added by kapilpaul 2 years ago.
Able to reproduce this issue in chrome
55393.diff (357 bytes) - added by alamgircsebd 2 years ago.
Created patch
BD68941D-6224-4D10-8057-DE5E96C14714.psd (3.0 MB) - added by kapilpaul 2 years ago.
comparison between break-all vs break-word
BD68941D-6224-4D10-8057-DE5E96C14714.jpg (461.2 KB) - added by kapilpaul 2 years ago.
comparison between break-all vs break-word
55393.modals.diff (1.4 KB) - added by sabernhardt 2 years ago.
adding a container to apply this style to original filenames within modal dialogs
55393.generic.diff (2.0 KB) - added by sabernhardt 2 years ago.
using a generic class name for the possibility of reusing it in unrelated admin situations
patch-media-library-more-details.png (170.1 KB) - added by sabernhardt 2 years ago.
applying break-word helps on the list view details screen (or "more details" in grid mode)
patch-modal-grid-view-details.png (17.2 KB) - added by sabernhardt 2 years ago.
using generic class name for grid view details dialog
patch-modal-post-insert-media.png (19.8 KB) - added by sabernhardt 2 years ago.
using generic class name for wrapping filename in post media inserter attachment details dialog (Classic Editor)
55393 Before Screen Shot 2022-06-02 at 14.53.58.png (80.7 KB) - added by circlecube 2 years ago.
Issue currently exists - before applying 55393.generic.diff
55393 After Screen Shot 2022-06-02 at 14.55.28.png (100.6 KB) - added by circlecube 2 years ago.
Issue fixed after applying 55393.generic.diff
55393.generic-2.diff (2.0 KB) - added by sabernhardt 2 years ago.
updating generic class name
Capture d’écran 2022-07-25 à 19.52.33.png (1.9 MB) - added by audrasjb 2 years ago.
Before patch
Capture d’écran 2022-07-25 à 19.55.20.png (1.9 MB) - added by audrasjb 2 years ago.
After patch

Change History (31)

#1 @costdev
2 years ago

  • Focuses css added
  • Milestone changed from Awaiting Review to 6.0
  • Version changed from 5.9.2 to 5.4

Issue Reproduction Report

Env

  • Web Server: Apache (Linux)
  • WordPress: 5.9.2
  • Browser: Firefox 98.0.1
  • OS: Windows 10
  • Theme: TT1 Blocks
  • Plugins: None activated.

Steps to reproduce

  1. Navigate to Media > Add New.
  2. Upload a large image with a long file name.
  3. Navigate to Media > Library.
  4. Click the "List Mode" icon.
  5. Click the "Edit" link for the image you uploaded.

To the right, the "Original image:" link should overflow the container.

Results

  1. Issue reproduced in Firefox. ✅
  2. Could not reproduce in Chrome (incl. Edge, Brave). ❌

Notes

  1. Introduced in 47202.
  2. word-break: break-word is used on the bolded "File name:" text above the link. This also works when applied to the "Original image:" link, however I personally think that word-break: break-all looks cleaner.
  3. Adding css focus for input on preference between break-word and break-all.
  4. 47202 introduced this link in the three locations below. The patch should be tested against all three of these.
  5. Milestoning for 6.0.
Last edited 2 years ago by costdev (previous) (diff)

#2 @costdev
2 years ago

  • Keywords needs-patch good-first-bug added

@kapilpaul
2 years ago

Able to reproduce this issue in chrome

@alamgircsebd
2 years ago

Created patch

#3 @alamgircsebd
2 years ago

  • Keywords has-patch added; needs-patch removed

@kapilpaul
2 years ago

comparison between break-all vs break-word

@kapilpaul
2 years ago

comparison between break-all vs break-word

#4 @kapilpaul
2 years ago

@costdev I have added a image for break-all vs break-word

break-word looks much cleaner to me.

The “word-break: break-all;” will break the word at any character so the result is to difficulty in reading whereas “word-wrap: break-word;” will split word without making the word not break in the middle and wrap it into next line.

#5 @sabernhardt
2 years ago

  • Milestone changed from 6.0 to 6.1

@sabernhardt
2 years ago

adding a container to apply this style to original filenames within modal dialogs

@sabernhardt
2 years ago

using a generic class name for the possibility of reusing it in unrelated admin situations

@sabernhardt
2 years ago

applying break-word helps on the list view details screen (or "more details" in grid mode)

@sabernhardt
2 years ago

using generic class name for grid view details dialog

@sabernhardt
2 years ago

using generic class name for wrapping filename in post media inserter attachment details dialog (Classic Editor)

#6 @sabernhardt
2 years ago

File names may need to wrap within the details dialogs, too, so I uploaded two options to consider more thorough approaches.

If the simpler approach is preferable, the same change was also suggested on #55819, which has good testing instructions.

#7 @peterwilsoncc
2 years ago

#55819 was marked as a duplicate.

#8 @peterwilsoncc
2 years ago

Additional props

@anantajitjg reported this issue on the duplicate ticket.

@afercia assisted with testing.

Committers: As the ticket was closed as a duplicate, please ensure these props are given when committing on this ticket.

@circlecube
2 years ago

Issue currently exists - before applying 55393.generic.diff

@circlecube
2 years ago

Issue fixed after applying 55393.generic.diff

#10 @circlecube
2 years ago

  • Keywords has-screenshots added

Tested first that I saw the issue in trunk, then tested 55393.generic.diff and confirmed that it fixes the issue. Nice job! Before and after screenshots attached above.

#11 follow-up: @SergeyBiryukov
2 years ago

If going with 55393.generic.diff, could we use word-wrap-break-word as the class name, just to match the CSS rule? I think that would be easier to remember than wrap-break-word.

#12 in reply to: ↑ 11 @rafiahmedd
2 years ago

Replying to SergeyBiryukov:

If going with 55393.generic.diff, could we use word-wrap-break-word as the class name, just to match the CSS rule? I think that would be easier to remember than wrap-break-word.

Yes, I think we should use word-wrap-break-word as the class name. It will be easy to understand its use case. Also, other developers can easily guess it.

@sabernhardt
2 years ago

updating generic class name

#13 @audrasjb
2 years ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Self-assigning for testing/review.

#14 @audrasjb
2 years ago

  • Keywords commit added

Looks great to me. Marking for commit.

#15 @audrasjb
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 53777:

Media: Prevent URLs from overflowing their container in the media editor.

This changeset also introduces the .word-wrap-break-word class which can be used to apply word-wrap: break-word to admin elements when needed.

Props mitogh, costdev, kapilpaul, alamgircsebd, sabernhardt, anantajitjg, afercia, circlecube, SergeyBiryukov, rafiahmedd, audrasjb.
Fixes #55393.

Note: See TracTickets for help on using tickets.