WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 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 7 years ago.
Patch to fix bad HTML form in media upload form

Download all attachments as: .zip

Change History (12)

@RanYanivHartstein7 years ago

Patch to fix bad HTML form in media upload form

comment:1 @westi7 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

comment:2 follow-up: @azaozz7 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.

comment:3 in reply to: ↑ 2 @westi7 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?

comment:4 @azaozz7 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.

comment:5 @azaozz7 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.

comment:6 @azaozz7 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

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

comment:8 follow-up: @RanYanivHartstein7 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.

comment:9 @westi7 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

comment:10 @ryan7 years ago

  • Component changed from General to Upload

comment:11 in reply to: ↑ 8 @azaozz6 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.