Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#17674 closed defect (bug) (fixed)

Formatting Bug on Importer Memory Limit Error Message

Reported by: JohnONolan Owned by: duck_
Milestone: 3.2 Priority: normal
Severity: minor Version: 3.1.4
Component: Import Keywords: has-patch commit
Focuses: Cc:


When running a WordPress import and hitting a memory limit, the error message displayed contains buggy formatting with html being rendered. Screenshot: http://cl.ly/7HyF

Attachments (1)

17674.diff (874 bytes) - added by JohnONolan 4 years ago.
Removes unneeded html which is being rendered

Download all attachments as: .zip

Change History (9)

4 years ago

Removes unneeded html which is being rendered

#1 @JohnONolan
4 years ago

  • Keywords has-patch added; needs-patch removed

#2 @JohnONolan
4 years ago

Screenshot of patch in action: http://cl.ly/7IMx

#3 @nacin
4 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to WordPress.org
  • Owner set to duck_
  • Status changed from new to assigned

Looks fine. esc_html() is probably good to keep depending on some variable error messages.

#4 @duck_
4 years ago

The patch John has attached is modifying WordPress core to remove the HTML tags rather than patching the importer. So should be 3.2 or Future Release.

esc_html() is probably good to keep depending on some variable error messages.

This is the problem I had with #16816 which fell into obscurity. Will close that since this has a proposed patch.

These errors are shown unescaped only in the inline uploaders in the editor -- see media_upload_form(), wp-admin/includes/media.php line 1465 -- and seem to be shown escaped in wp-admin/async-upload.php though I do not know where or if the user will ever see the latter.

#5 @JohnONolan
4 years ago

My thought behind the patch was mainly that the HTML tags weren't actually necessary in this context? Point taken about the larger issue of patching the importer though!

#6 @nacin
4 years ago

  • Milestone changed from WordPress.org to 3.2

Let's figure this out?

#7 @westi
4 years ago

I think stripping the HTML here makes the messages less useful so I did a quick audit of how core treats these messages:

  • wp-admin/media-new.php - redirects on any error to wp-admin/upload.php?message=3 and shows a poor generic message - "Error saving media attachment."
  • wp-admin/post-new.php - Media upload HTML - displays the HTML message (in red!)
  • wp-admin/post-new.php - Media upload Flash - displays the HTML message with the tags in it :(

So it seems at the moment we have inconsistent behaviour in core.

It seems like stripping out the HTML from these messages is the most compatible solution for 3.2

#8 @markjaquith
4 years ago

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

In [18193]:

Remove code formatting from uploaded file size error messages, for now. props JohnONolan. fixes #17674

Note: See TracTickets for help on using tickets.