Make WordPress Core

Opened 9 months ago

Last modified 9 months ago

#58416 new enhancement

Media Title field should soft wrap in Media Modal: For better readability/editing of long titles.

Reported by: abitofmind's profile abitofmind Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: commit has-patch has-screenshots has-testing-info needs-testing needs-unit-tests
Focuses: administration Cc:

Description

Usability Issue:

  • In the Media Modal the Title field currently is an <input type=text> and as such is single line only.
  • Scrolling a ~50 character long title within a ~20 char text field sucks.
  • Soft wrapping would really benefit longer media titles in terms of instant readability and easy editing.

Solution Approaches:

  1. Leaving it a <input type=text> and tweak it to soft wrapping with CSS only.
  1. Alternative approach: Use a <textarea> with its built in soft-wrapping and resize-ability, integrated with contemporary responsive web design techniques, and ensure that the user can not unintentionally insert linebreaks.
  • This seems to be the viable solution.
  • Googled & DuckDuck'ed CMS-agnostic, but by coincidence found WordPress specific solution even.
  • JS which intercepts ENTER control characters.
  • To be ultra safe (does it need to be?) this would also need server-side validation (in case JS is bypassed somehow (un)intentionally).

Attachments (6)

WordPress--Media-title-field-with-soft-wrapping.png (191.5 KB) - added by abitofmind 9 months ago.
Media Textarea Linebreaks 1 in Modal.png (244.8 KB) - added by abitofmind 9 months ago.
Media Textarea Linebreaks 2 in Image Block Insert Dialog.png (210.0 KB) - added by abitofmind 9 months ago.
Media Textarea Linebreaks 3 in Block Editor Inspector Panel.png (285.8 KB) - added by abitofmind 9 months ago.
Media Textarea Linebreaks 4 in Frontend Output correctly in alt text without any linebreaks.png (213.9 KB) - added by abitofmind 9 months ago.
58416.patch (1.4 KB) - added by mrinal013 9 months ago.
These codes do soft wrap the Media Title field in media modal, also prevent to press Enter from keyboad in the Media Title Field.

Download all attachments as: .zip

Change History (30)

This ticket was mentioned in PR #4516 on WordPress/wordpress-develop by @mrinal013.


9 months ago
#1

  • Keywords has-patch added

Soft wrap the Media Title field in media modal: For better readability/editing of long titles.

Trac ticket: 58416

#2 @ankitmaru
9 months ago

Hi, @abitofmind Welcome back to WordPress Trac!

LGTM

porg commented on PR #4516:


9 months ago
#3

B/c if it is a standard HTML textarea then the user can (un)intentionally insert line breaks.

Is your patch avoiding linebreaks client or server side?

@mrinal013 commented on PR #4516:


9 months ago
#4

Hello @porg,
Thanks for your hint.
Now, this change prevents (un)intentional line breaks.

#5 @mrinal013
9 months ago

  • Focuses administration added

@mrinal013 commented on PR #4516:


9 months ago
#6

Hello @porg ,

Now these changes have been able to

  1. soft-wrap the title
  2. do server-side validation if a user (un)intentionally uses a line break
  3. do server-side validation if a user (un)intentionally use any tags in the title of the attachment

#7 @mrinal013
9 months ago

  • Focuses privacy added

#8 @abitofmind
9 months ago

Do you have a running instance of a WordPress dev/test server with this your fixes?

I'd love to try it out and give you feedback.

#9 @abitofmind
9 months ago

privacy tag: IMHO this issue is not privacy related at all. The line break avoidance features you added is about data integrity achieved by client and server side validation. At maximum I'd say is that "security" is ensured by the server side validation. But maybe in WordPress Core context "privacy" is used as an umbrella term, with a loose meaning? Then ok. But in the true meaning of the word, I'd say "privacy" is inappropriate. "admin" as the only tag for this feature would be enough as I see it.

#10 @mrinal013
9 months ago

  • Focuses privacy removed
  1. Yes, I have an instance of this WordPress, which is basically a fork of [wordpress-develop](https://github.com/WordPress/wordpress-develop)
  1. Yes, I think so about the 'privacy' tag. I misunderstood that, this field is about security so I tagged the 'Privacy '. However, this can be removed.

#11 @mrinal013
9 months ago

  • Keywords has-screenshots has-testing-info added

Test Report


Environment

  • Docker - v20.10.21
  • WordPress - 6.3-alpha-55505-src
  • Chrome Version - 113.0.5672.127 (Official Build) (64-bit)
  • Edge Version - 113.0.1774.57 (Official build) (64-bit)
  • OS - Windows 11
  • Active theme: Twenty Twenty Three
  • PHP - 7.4.33
  • MySQL - Ver 15.1 Distrib 10.4.27-MariaDB, for Win64 (AMD64), source revision 0946c99e7d6f7ac9dfcf3e60dae6ae85161d5ef2

Steps to test

  • Go to WordPress Dashboard
  • Go to "Media" from the sidebar, then "Add New"
  • Upload the image by clicking on the "Select File" button
  • Again go to the "Media" page from the sidebar
  • Get the attachment detail by clicking on this image
  • Here find out the "Title" field under the "Alternative Text" field
  • Before fetching this PR, the title field is simply one-line and is not suitable for a long sentence
  • After applying this PR, the title field is multiline and suitable for a long sentence

Before


https://prnt.sc/fhj9UYGiDJV3

After


https://prnt.sc/_BB0NGmMahCc

Last edited 9 months ago by mrinal013 (previous) (diff)

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


9 months ago

#13 @mrinal013
9 months ago

  • Keywords needs-testing added

#14 @mrinal013
9 months ago

  • Keywords needs-unit-tests added

#15 @abitofmind
9 months ago

Thanks for your test report!

Could you please send me the credentials to your WP test instance via Slack PM, so that I as a software tester can test it myself independently from you?

If not, then please you run the following tests and record them on video and while your test steps clearly state aloud in the audio or show via key visualizer what you currently do on your keyboard.

Test plan for this feature

For manual testing. And possibly also the blueprint for the unit tests:

  1. Type long enough text that the text auto wraps. Remove text again to see it wrapping to fewer lines or a single line only eventually.
  1. Resize the window so that we see how the textarea auto-resizes within the available layout width.
  1. Resize the textarea manually.
  1. Demonstrate the client side linebreak avoidance.
  • a) Press ENTER at the start of the string, middle of the string, end of the string.
  • b) A single time. And then again multiple consecutive times.
  • c) Try all sort of new line control characters as used by the different operating systems (LF on Unixes, CR MacOS Classic, CR-LF Windows).
  • d) Also try with the ENTER vs. RETURN key.
  • e) Also try inserting a multi line clipboard via ctrl-C (cmd-C on Mac).
  • f) Try also to insert other forms of (unintentional) linebreaks, like \nor <br> or <p>Start of title</p><p>and later part of the title</p>. Not even mimicking bad user intents, just think copy paste scenarios going wrong.
  1. Demonstrate the server side linebreak avoidance:
  • a) Try to play an intentional bad actor who submits the line break via HTML form manipulation (GET and POST) or DOM manipulation (Inspect with WebTools and manipulate linebreak there) and then show us the system response (expected: validation message near field).
  • b) Also simulate how that may happen without bad intent with a browser back, browser forward event and a subsequent form re-submission, with any of the input from test steps 4a-f.

#16 @joedolson
9 months ago

  • Version changed from 6.2.2 to 3.5

We have a precedent for changing a text field from a text input to textarea input, which we did for the alt attribute field on #50066. This can follow the same plan; all we really need to do is sanitize the input using sanitize_text_field instead of sanitize_textarea_field, so that line breaks won't be retained. We just need to make sure to comment on that in the code so it's clear why.

Changing version to 3.5, which is when the add media panel was introduced, since this is specifically referring to the modal. However, the use of a text field for the media title field probably dates back to the first support for media in WordPress.

#17 @abitofmind
9 months ago

@mrinal013

  1. Sounds like an offer/hint to re-use code. 🙂

@joedolson

  1. I guess for the server side part of this issue here, because WordPress at v3.5 likely had probably not too much client side JavaScript monitoring/validations, I assume.
  1. Just to be sure: The sanitize_text_field and sanitize_textarea_field you talk about are custom for the use case purpose of our ticket here and have nothing to do with sanitize_file_name and all the hooks this has which plugins like Media File Renamer? Asking because my requested feature should in no way intervene with sanitize_file_name().

#18 @abitofmind
9 months ago

@joedolson

Ad 2) I now studied the very long ticket history (over the last 3 years) and a lot of screenshots too. So this issue made it from the Classic Editor, later into the Block Editor. Just studying the screenshots (not the code, would mostly not understand it anyhow), this looks to be 1:1 the same use case as ours here, just for the Media Title instead of the Alt Text! Would be great if that could be simply re-used !

4) Now I have some open questions, see the following annotated screenshots. Hope you can answer / react to those.

#19 @mrinal013
9 months ago

Hello @abitofmind ,

Thanks for your hint. I will push ASAP.

However, I try to add the following code in src/wp-includes/media-editor.js, but can't find out a way to build it on my end. So, I will give a time to implement the script.

// Prevent Enter in title textarea
$(document.body)
    .on( 'keypress', '#attachment-details-two-column-title', function( event ) {
	if ( event.key == 'Enter' ) {
	    return false;
	}
});

This script will prevent Enter keyboard event. For that, the meta_value for the attachment in the database is similar to the textarea field. Will it solve the issue?

As the screenshot of the alt text field, this is also doing irregular behavior.

#20 @abitofmind
9 months ago

@joedolson Could you please answer the questions from the annotated screenshots? Again here:

1) In the Media Modal the ALT Text textarea allows linebreaks client side, and even restores them when re-opening that media file later.

a) Where/how are those linebreak persisted? In the database only? Or is the browser's localstorage? Or is my web browser's web form autofilling playing me an illusion here? (I don't think so, but have not investigated it either).

b) Is the ALT Text textarea in the other UIs/contexts (Image Insertion Dialog in Block Editor + Inspector Panel in Block Editor) showing the data as-is (they landed in the database sanitized without linebreaks) or is a runtime linebreak to whitespace transformation happening?

@ all

2) Allowing linebreaks client side in the ALT Text, is a good UX for structuring long text onto multiple lines. As I wrote in the screenshot annotation.

a) Question: If the text is One\n\nTwo\n\nThree client side how is this persisted? As is? Or with \n+ being transformed to a uniform singular whitespace? Again If I get an answer to 1a) I would have fewer speculative followup questions.

b) UX discussion: For the "Media Title" which anyhow is only for internal use (browsing/searching your library) and does not make it into the frontend output (for many versions now). Media Titles can be a bit longer too. But by tendency they are less long than an Alt Text (if it indeed gives a full/rich visual description). It seems more correct to disallow linebreaks also client side, to more explicitly communicate to the user "this is a title, which serves for lookups (almost a unique identifier), and which is a single line only" potentially with a length limited lower to than what's allowed for the alt text (I dunno).

TL;DR: So disallowing manual linebreaks but allowing soft wrapping seems to be the right look & feel for the ALT Text textarea.

#21 @mrinal013
9 months ago

@abitofmind ,

  1. "Allowing linebreaks client side in the ALT Text, is a good UX for structuring long text onto multiple lines" - is a good idea to me. However, recently I work on a website where attachment title is used. However, attachment title could be more helpful, if it can allow line-break.

#22 @joedolson
9 months ago

1) The state management in the media modal is persistent as long as you're in the same session. If you reload the screen, however, it'll show you the saved version. It's only persistent locally; that version is never saved.

2) It won't be shown elsewhere with line breaks, as it's never been saved that way.

3) Line breaks need to be stripped out for backwards compatibility, because historically this field was used for image title attributes and there are still themes and plugins out there that expect this. I don't think we should change the structure of the data, particularly given that it's usually not front-end facing content. I don't see any reason to change the limits on length; it's stored the same way as any post title, and has the same limits. (65,535 characters).

#23 @abitofmind
9 months ago

@joedolson

  • Ad 1-3) Thanks for describing. All working as I had hoped. Nice! The linebreaks only live in some session variables and only the sanitized variant gets persisted to the database and called by all other UIs. Perfect!
  • Ad 3) The sanitation works fine. Whatever amount of whitespace linebreaks whitespace \w*\n+\w* gets normalized to a single whitespace. I tested multiple variations now, and they all brought a consistent result.

@mrinal013 or @joedolson yourself

  • Please use this solution as-is and apply it for the Media Title field to get a textarea with this look & feel.

@mrinal013

  1. As @joedolson put it very clearly: Line breaks are forbidden in both the Media Title (aka Attachment Title aka post_title for media entities) and the ALT Text.
  2. Without knowing your intended application in detail, possibly https://wordpress.org/plugins/media-library-assistant/ can has you covered!
    • With it you can map between or among any of those fields: EXIF ↔︎ IPTC ↔︎ WP standard media fields ↔︎ WP custom fields:
    • On upload, on Media Edit events, and also on demand in a bulk interface.
    • And you can perform powerful multi pass RegEx find/replace operations etc.
    • My application as an example
    • It has a very oldschool UI, but it is very powerful.
    • Sadly there is no online documentation available, but the line documentation, even if sometimes very technical/dry, covers really everything! Long learning curve, but then very satisfactory and reliable.
    • Potentially you can also use a special symbol like a bullet "•" in one field which does not support linebreaks into another field which supports line breaks, and there replace it as a line break, the possibilities are endless.

@mrinal013
9 months ago

These codes do soft wrap the Media Title field in media modal, also prevent to press Enter from keyboad in the Media Title Field.

#24 @mrinal013
9 months ago

  • Keywords commit added
Note: See TracTickets for help on using tickets.