WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#7293 closed defect (bug) (wontfix)

Bad HTML form in media upload form

Reported by: RanYanivHartstein Owned by: westi
Milestone: Priority: normal
Severity: normal Version: 2.6
Component: Upload Keywords: needs-patch, rtl, I18N
Focuses: Cc:

Description

The media upload form in wp-admin/wp-includes/media.php for uploading images has one form field defined in different HTML than all the rest. This includes using an image instead of abbreviated text to mark a field as required, and unnecessarily wrapping one form field in paragraph tags.

Attachments (1)

media.php.r8328.diff (1.1 KB) - added by RanYanivHartstein 8 years ago.
Patch to fix bad HTML form in media upload form

Download all attachments as: .zip

Change History (12)

@RanYanivHartstein
8 years ago

Patch to fix bad HTML form in media upload form

#1 @westi
8 years ago

  • Keywords dev-reviewed commit added
  • Milestone set to 2.7
  • Owner changed from anonymous to westi
  • Status changed from new to assigned
  • Version set to 2.6

#2 follow-up: @azaozz
8 years ago

This form replaces TinyMCE's Insert Image dialog and behaves in similar way. As soon as an image URL is entered, it loads the image in the background to check that it's accessible (some sites have "hotlinking protection") and to get the exact width of the image (needed for adding captions).

There's a small icon that gives feedback when the entered URL is not accessible or mistyped. It replaces the "Required" asterisk. For that to work smooth, the asterisk is an icon too.

The extra <p> tag is there to add a margin between the required and the optional fields in the form.

#3 in reply to: ↑ 2 @westi
8 years ago

  • Keywords dev-reviewed commit removed

Replying to azaozz:

This form replaces TinyMCE's Insert Image dialog and behaves in similar way. As soon as an image URL is entered, it loads the image in the background to check that it's accessible (some sites have "hotlinking protection") and to get the exact width of the image (needed for adding captions).

There's a small icon that gives feedback when the entered URL is not accessible or mistyped. It replaces the "Required" asterisk. For that to work smooth, the asterisk is an icon too.

The extra <p> tag is there to add a margin between the required and the optional fields in the form.

Thanks for the explaination.

Does that make this WONTFIX?

#4 @azaozz
8 years ago

There are few HTML validation errors that weren't mentioned by the reporter, but since the title is "Bad HTML...", perhaps can fix them here instead of a new ticket.

#5 @azaozz
8 years ago

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

(In [8361]) Fix invalid HTML and remove duplicate fields. Fixes #7293 for trunk.

#6 @azaozz
8 years ago

  • Milestone changed from 2.7 to 2.6.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-open for 2.6.1

#7 @azaozz
8 years ago

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

(In [8362]) Fix invalid HTML and remove duplicate fields. Fixes #7293 for 2.6.

#8 follow-up: @RanYanivHartstein
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This still doesn't explain why an <abbr> tag is used to show the asterisk in one form field, whereas in others an image is used.

Furthermore, using a <p> to add spacing is bad form - it would be much better to simply increase the bottom margin of the relevant element.

The reason I opened this in the first place is because building the form field this way totally breaks when applying the RTL styles, and it seemed like a better idea to fix the root of the problem for all users instead of applying lines and lines of css hacks to override this just for RTL users.

#9 @westi
8 years ago

  • Keywords needs-patch rtl I18N added; has-patch removed
  • Milestone changed from 2.6.1 to 2.6.2

2.6.1 has been released, moving to 2.6.2 milestone

#10 @ryan
8 years ago

  • Component changed from General to Upload

#11 in reply to: ↑ 8 @azaozz
7 years ago

  • Milestone 2.7 deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Replying to RanYanivHartstein:

This still doesn't explain why an <abbr> tag is used to show the asterisk in one form field, whereas in others an image is used.

Because the image is used to show the state of the entered URL.

Furthermore, using a <p> to add spacing is bad form - it would be much better to simply increase the bottom margin of the relevant element.

<p> defines new paragraph there.

Note: See TracTickets for help on using tickets.