Make WordPress Core

Opened 22 months ago

Closed 18 months ago

Last modified 16 months ago

#62606 closed defect (bug) (duplicate)

Dismiss Button in Media Upload Error Message Not Working

Reported by: sukhendu2002 Owned by:
Priority: normal Milestone:
Component: Media Version: 6.4
Severity: normal Keywords: has-screenshots has-patch has-test-info
Cc: Focuses: javascript

Description

When an unsupported file is uploaded via the "Upload New Media" section, an error message is displayed. However, the "Dismiss" button on the error message does not function as expected.

The issue occurs because the onclick attribute on the dismiss button is stripped out due to wp_admin_notice() using wp_kses_post() internally.

Steps to Reproduce:

  • Navigate to Media > Add New Media File in the admin dashboard.
  • Attempt to upload an unsupported file type (e.g., a .exe file).
  • Observe that an error message appears.
  • Click the "Dismiss" button on the error message.

Attachments (3)

media-upload-error-dismiss-before-fix.mov (2.8 MB ) - added by sukhendu2002 22 months ago.
dissmiss_errors.mp4 (3.4 MB ) - added by rinkalpagdar 22 months ago.
62606-echo.diff (410 bytes ) - added by sabernhardt 21 months ago.
alternative option: echo wp_get_admin_notice()

Change History (12)

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


22 months ago
#1

  • Keywords has-patch added

This PR fixes an issue where the dismiss button in the media upload error messages was not working.

Trac ticket: https://core.trac.wordpress.org/ticket/62606

https://github.com/user-attachments/assets/edf67b4b-2c3b-4df0-89f0-da6c7997dc87

#2 @abcd95
22 months ago

  • Keywords has-testing-info added

Reproduction Report

Description

This report validates that the issue can be reproduced.

Environment

  • WordPress: 6.7.1
  • PHP: 8.1.29
  • Server: nginx/1.16.0
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.29)
  • Browser: Chrome 131.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.0

Actual Results

✅ Error condition occurs.

Supplemental Artifacts

Screencast - https://utfs.io/f/vtiKpIr2gd0c32cyGIoXiUKalkW6HNjG2YsI0Jq4mdVb1EfM

#3 @azaozz
22 months ago

  • Milestone Awaiting Review6.8
  • Version 6.7.16.4

As far as I see the PR will fix this. However there is already a way to make admin notices dismissable, see `makeNoticesDismissible()` in common.js. Could that be used instead?

#4 @rinkalpagdar
22 months ago

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.8-alpha-59274-src
  • PHP: 8.2.25
  • Server: nginx/1.27.2
  • Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.25)
  • Browser: Chrome 131.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ❌ Error condition does not occur (not reproduced).

Additional Notes

  • I have added a screencast for this.

Supplemental Artifacts

https://core.trac.wordpress.org/attachment/ticket/62606/dissmiss_errors.mp4

@sabernhardt
21 months ago

alternative option: echo wp_get_admin_notice()

#5 @sabernhardt
21 months ago

The async upload already escapes the filename and error message, so it could switch to echo wp_get_admin_notice(). It would not need the action or further sanitization in wp_admin_notice().

I was not successful at making the notice consistent with other dismissible notices (comment:3), but if someone else can, that could be an improvement to replace the onclick attribute.

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


18 months ago

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


18 months ago

#8 @peterwilsoncc
18 months ago

  • Milestone 6.8
  • Resolutionduplicate
  • Status newclosed

This was reported in #60074, so I am closing it as a duplicate.

The good news is that a fix was committed against the other ticket resolving the issue a few days ago, see [59986], so it will be resolved in WordPress 6.8.

#9 @wordpressdotorg
16 months ago

  • Keywords has-test-info added; has-testing-info removed
Note: See TracTickets for help on using tickets.