WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 9 days ago

#48028 reopened defect (bug)

Media Library 'featured image' dialogue missing link text that describes 'opens in new tab'.

Reported by: anevins Owned by: antpb
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-screenshots wpcampus-report has-patch
Focuses: accessibility Cc:
PR Number:

Description

The "Media Library" panel of the "Featured Image" modal dialog contains a link labelled with "Edit image". This link takes users to a new window/tab, but that link does not inform users that it opens a new window or tab.

A link does not naturally convey itself to assistive technology that it opens in a new tab without link text that describes it.

It is important to convey that a link opens in a new tab because of the change of context may not be clear to everyone.

Suggested solution

  • Write more descriptive link text that describes the link opening in a new tab;
  • Utilise the class of 'screen-reader-text' in CSS to hide that text (you can wrap the text in <span class="screen-reader-text">).

Example of solution

<a class="edit-attachment" href="..." target="_blank">
  Edit Image
  <span class="screen-reader-text">(Opens in a new window)</span>
</a>

This problem was originally raised on ticket #47137 and has been separated out here to reduce complexity.

Attachments (3)

57185883-4f359f80-6e89-11e9-9e23-903e587ff978.png (362.5 KB) - added by anevins 6 weeks ago.
48028.patch (680 bytes) - added by fblaser 6 weeks ago.
This should do it.
48028.2.patch (1.1 KB) - added by antpb 3 weeks ago.
Fixes edit state to open in modal for image block, featured frame. Work in progress. only for discussion.

Download all attachments as: .zip

Change History (30)

@fblaser
6 weeks ago

This should do it.

#1 follow-up: @fblaser
6 weeks ago

  • Keywords has-patch added; needs-patch removed

Greetings from WordCamp Zürich Contributor's day!

I've just submitted a patch for this bug. Hope this helps.

#2 in reply to: ↑ 1 @w3rkjana
6 weeks ago

Replying to fblaser:

Greetings from WordCamp Zürich Contributor's day!

I've just submitted a patch for this bug. Hope this helps.

Installed, checked patch. Text is present, great thanks.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


6 weeks ago

#4 @afercia
6 weeks ago

  • Focuses accessibility added
  • Milestone changed from Awaiting Review to 5.3

Hello Zürich! Thanks for the patch. Going to move this ticket to the 5.3 milstone and set the "Accessibility" focus.

Last edited 6 weeks ago by afercia (previous) (diff)

#5 @afercia
5 weeks ago

  • Keywords good-first-bug removed

Looking a bit more in depth into this issue, it appears it's a bit more complicated than expected.

In the Classic Editor the "Edit Image" link doesn't open a new tab. Even if it does have a target="_blank" attribute, the default action is prevented and the media modal updates its content to display the image editor. This is intended behavior. See https://core.trac.wordpress.org/browser/trunk/src/js/media/views/attachment/details.js?rev=45561&marks=189-199#L180

Instead, in Gutenberg and in the Customizer it opens a new tab. This happens because editState is undefined and I'm not sure it's the intended behavior to start with.

Would anyone from the Media or Editor team clarify if there's any reason why the Image Editor should open in a new tab in the context of Gutenberg (and the Customizer)?

Pinging some of the related components maintainers: @antpb @mikeschroder @azaozz @iseulde

This ticket was mentioned in Slack in #core-media by mike. View the logs.


5 weeks ago

#7 @afercia
5 weeks ago

To further clarify: seems to me editState is only defined in the post frame but Gutenberg (and I guess the Customizer as well) uses a different frame, where editState is undefined.

Also, because of that, the frame gets in a dirty state and the Edit Image link disappears.

This might imply to review the way the media frames are used in the context of Gutenberg and the Customizer, as seems they're not fully functional.

#8 @antpb
5 weeks ago

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

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #accessibility by antpb. View the logs.


4 weeks ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 weeks ago

#12 @antpb
3 weeks ago

I've created a new Gutenberg github issue that covers fixing this ticket. Instead of adding a "opens in new tab" alert, we need to fix the media modal. The opening in a new tab is actually a bug that needs to be resolved in the MediaUpload component of Gutenberg. Currently, it does not initialize the logic required to load the edit image state inside the modal.

I have a PR ready to review here: https://github.com/WordPress/gutenberg/pull/17641

Here also is the Gutenberg Github Issue : https://github.com/WordPress/gutenberg/issues/17642

#13 @afercia
3 weeks ago

  • Keywords needs-patch added; has-patch removed

The opening in a new tab is actually a bug that needs to be resolved in the MediaUpload component of Gutenberg.

@antpb I'm not fully sure, because the media modal can be used with various "frames" also in core. When the used frame is not the "post" one, the logic required to load the edit image state inside the modal is missing as well.

For example, if I'm not wrong, the Customizer uses the "select" frame which lacks that logic. This is a core bug and needs to be fixed in the media views.

Also, it's pretty common for plugins to use the "select" frame.

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


3 weeks ago

#15 @antpb
3 weeks ago

@afercia Thanks for that info! I had no clue the customizer had this behavior also.

@joemcgill voiced the same suggestion you did on making this a global change for the select frame. I'll need to do some digging to find a fix there. My initial findings on that approach was that the editState function has a dependency on the state of the frame which is different in some cases (featured image and gallery frames) and also a need to filter the library by the allowedTypes prop that is passed from the MediaUpload component. Here's a link to that code that may help illustrate: https://github.com/WordPress/gutenberg/pull/17641/files#r329723943

more info to come later in the week! :)

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


3 weeks ago

@antpb
3 weeks ago

Fixes edit state to open in modal for image block, featured frame. Work in progress. only for discussion.

#17 @antpb
3 weeks ago

I've added a new patch that fixes the "edit image" link for the image block flow. Customizer is still strangely opening in a new tab so I am still investigating that. I've also identified another bug when we get the edit image button working. When you click "back" the frame ends up in a weird state where there is a sidebar and the select media button is blank.

#18 @marybaum
2 weeks ago

  • Milestone 5.3 deleted
  • Resolution set to invalid
  • Status changed from assigned to closed

#19 @garrett-eclipse
2 weeks ago

  • Milestone set to Future Release
  • Resolution invalid deleted
  • Status changed from closed to reopened

#20 @mikeschroder
2 weeks ago

  • Milestone changed from Future Release to 5.3

Just a quick note that I think this one is fine to land in 5.3 still, if anyone would like to work on a patch between now and October 15 for RC.

Going to add it back for tracking. We can always punt before RC if needed.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


2 weeks ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


13 days ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


12 days ago

#24 @antpb
12 days ago

  • Keywords has-patch added; needs-patch removed

New update! Sorry for the delay but I have been slowly plugging away at finding the needles in this haystack. :)

The latest patch is all that is needed in Core to prepare us to fix this issue in Gutenberg. I have updated a Gutenberg PR that fixes image block, gallery block, inline images, and featured image flow. https://github.com/WordPress/gutenberg/pull/17641/

Customizer media frames are still opening in a new page and that is a mystery to me that I think would be best set in a new ticket as this ticket grew a much wider scope than we expected.

With this patch committed, and the PR above merged, we will get this mostly fixed.

#25 @joemcgill
12 days ago

@antpb 48028.2.patch looks good to me. Feel free to commit this one separately from a future patch that will address the holdover Customizer issues. Remember to run grunt precommit before committing to ensure all the right files get built.

#26 @antpb
12 days ago

In 46461:

Media: Adds Edit Image controller to Media Library select frame.

This adds the necessary state for EditImage views in the select frame and listeners for setting that view.

Props fblaser, w3rkjana, afercia, antpb.
See #48028.

#27 @davidbaumwald
9 days ago

  • Milestone changed from 5.3 to Future Release

With 5.3 RC1 releasing today and work still left on this ticket, it is being moved to Future Release. If any committer feels this can be worked in quickly for 5.3 or can assume ownership in 5.4, feel free to move it back up.

Also, @antpb mentioned a related Gutenberg PR that would resolve the remaining issues. The PR is https://github.com/WordPress/gutenberg/pull/17641 If this can be merged upstream, the rest of this ticket could be closed out.

Note: See TracTickets for help on using tickets.