Make WordPress Core

Opened 6 months ago

Closed 2 months ago

Last modified 8 days ago

#62606 closed defect (bug) (duplicate)

Dismiss Button in Media Upload Error Message Not Working

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

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 6 months ago.
dissmiss_errors.mp4 (3.4 MB) - added by rinkalpagdar 6 months ago.
62606-echo.diff (410 bytes) - added by sabernhardt 5 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.


6 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
6 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
6 months ago

  • Milestone changed from Awaiting Review to 6.8
  • Version changed from 6.7.1 to 6.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
6 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
5 months ago

alternative option: echo wp_get_admin_notice()

#5 @sabernhardt
5 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.


2 months ago

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


2 months ago

#8 @peterwilsoncc
2 months ago

  • Milestone 6.8 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

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
8 days ago

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