WordPress.org

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:

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

Download all attachments as: .zip

Change History (9)

@JohnONolan4 years ago

Removes unneeded html which is being rendered

comment:1 @JohnONolan4 years ago

  • Keywords has-patch added; needs-patch removed

comment:2 @JohnONolan4 years ago

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

comment:3 @nacin4 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.

comment: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.

comment:5 @JohnONolan4 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!

comment:6 @nacin4 years ago

  • Milestone changed from WordPress.org to 3.2

Let's figure this out?

comment:7 @westi4 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

comment:8 @markjaquith4 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.