Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#17674 closed defect (bug) (fixed)

Formatting Bug on Importer Memory Limit Error Message

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

Description

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 12 years ago.
Removes unneeded html which is being rendered

Download all attachments as: .zip

Change History (9)

@JohnONolan
12 years ago

Removes unneeded html which is being rendered

#1 @JohnONolan
12 years ago

  • Keywords has-patch added; needs-patch removed

#2 @JohnONolan
12 years ago

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

#3 @nacin
12 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_
12 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
12 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
12 years ago

  • Milestone changed from WordPress.org to 3.2

Let's figure this out?

#7 @westi
12 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
12 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.