Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#13383 closed task (blessed) (fixed)

Validate the HTML in the admin area

Reported by: nacin's profile nacin Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Administration Keywords: needs-patch
Focuses: Cc:

Description

Per the dev chat today: Check for and correct invalid HTML in the admin area.

We're not concerned about ARIA or autocomplete -- we're concerned about invalid HTML that reveals improper markup (such as unclosed or improperly nested tags), as that usually leads to larger display issues when rendering the page under some or all conditions.

Small patches from anyone willing to join in. (Stick to small patches that way multiple contributors don't duplicate efforts.) One of us will commit as soon as we see them. Be sure to post a comment in addition to the patch to trigger a notification.

Don't forget about multisite. :-)

Attachments (24)

cb1.diff (252 bytes) - added by jshreve 14 years ago.
end tag for "tr" omitted in custom-background.php
default-widgets1.diff (1.5 KB) - added by jshreve 14 years ago.
two unclosed 'p' tags
media1p.diff (345 bytes) - added by jshreve 14 years ago.
echoing a </td></tr> instead of adding it to output
13383-theme-editor.diff (3.9 KB) - added by Utkarsh 14 years ago.
Fixes <div> inside <a> in Theme Editor.
13383-custom-background.diff (1.2 KB) - added by Utkarsh 14 years ago.
Fixes several validation errors on custom background page.
13383-custom-header.diff (1.4 KB) - added by Utkarsh 14 years ago.
Fixes validation errors on custom header page.
13383-custom-background.2.diff (1.1 KB) - added by Utkarsh 14 years ago.
Custom background patch refreshed against r14626
13383-remove-trailing-space-in-label.diff (5.4 KB) - added by zeo 14 years ago.
Removes unnecessary trailing space in LABEL
13383-use-lowercase.diff (1.8 KB) - added by zeo 14 years ago.
Standardize FORM id for upload to use lowercase with hyphen. See media.php: <?php echo $type; ?>-form
13383-ms-sites.diff (1.7 KB) - added by Utkarsh 14 years ago.
Fixes validation errors on ms-sites.php
13383-ms-users.diff (721 bytes) - added by Utkarsh 14 years ago.
Fixes validation errors on ms-users.php
escurl.diff (1.6 KB) - added by jshreve 14 years ago.
adds missing esc_url calls, fixes many validation errors
13383-no_duplicate_function_call.patch (566 bytes) - added by TobiasBg 14 years ago.
Patch to remove duplicate function call of [14628], see #comment:15
13383-dont-wrap-div-with-p-tags.diff (1.0 KB) - added by zeo 14 years ago.
Media Library: don't wrap div with p tag.
13383-escape-preview.diff (1.9 KB) - added by zeo 14 years ago.
escape preview link
13383-ms-options.diff (4.8 KB) - added by Utkarsh 14 years ago.
Fixes all validation errors on ms-options.php
13383-add-backlash.diff (704 bytes) - added by zeo 14 years ago.
JS: add backlash
13383.network.diff (2.3 KB) - added by nacin 14 years ago.
untested
13383-ms-sites.2.diff (1.2 KB) - added by Utkarsh 14 years ago.
Fixes all validation errors on ms-sites.php
msnonce.diff (3.3 KB) - added by jshreve 14 years ago.
fixes some of the multiple nonce errors on multi isite
mssites.diff (1.2 KB) - added by jshreve 14 years ago.
Fixes additional errors on the editblog screen in ms-sites.php
themeeditor.valid.patch (1.3 KB) - added by ocean90 14 years ago.
newpost.valid.patch (1.4 KB) - added by ocean90 14 years ago.
media_upload_for_nojs.patch (1.4 KB) - added by ocean90 14 years ago.

Download all attachments as: .zip

Change History (67)

#1 @Viper007Bond
14 years ago

  • Keywords needs-patch added
  • Version set to 3.0

@jshreve
14 years ago

end tag for "tr" omitted in custom-background.php

#2 @jshreve
14 years ago

  • Cc justin.shreve@… added

I'm going to jump in on this as much as I can (it's finals week). I found a missing tr in custom-background.php that the above patch fixes.

@jshreve
14 years ago

two unclosed 'p' tags

@jshreve
14 years ago

echoing a </td></tr> instead of adding it to output

#3 follow-up: @jshreve
14 years ago

Sorry for the multiple patches - doing this in between other things.

The last one was causing an error under circumstances because a </td></tr> set was being echoed instead of added to the output string for get_media_item.

#4 @nacin
14 years ago

(In [14617]) Add closing tr tag to custom-background.php. Close paragraph tags in the recent comments and posts widget controls. props jshreve, see #13383.

#5 @nacin
14 years ago

(In [14618]) Append closing td/tr to output instead of directly echoing it. props jshreve, see #13383.

#6 in reply to: ↑ 3 @Viper007Bond
14 years ago

Replying to jshreve:

Sorry for the multiple patches - doing this in between other things.

Nothing wrong with multiple patches. See original ticket description. :)

#7 @nacin
14 years ago

Also, mark off files you've tested if you can, and mention whether or not you tested it on multisite. Just so we don't duplicate efforts.

#8 @dremeda
14 years ago

I am falling asleep. theme-editor.php is throwing <div class='highlight'> inside of an anchor. Line 184 I think.

I will get to it in the next day or two unless someone knocks it out.

#9 @dremeda
14 years ago

  • Cc dre@… added

@Utkarsh
14 years ago

Fixes <div> inside <a> in Theme Editor.

#10 @Utkarsh
14 years ago

  • Cc admin@… added

dremeda, just submitted a patch for that :)

#11 @nacin
14 years ago

(In [14625]) Use valid HTML in the theme editor. span instead of div inside an anchor. props Utkarsh, see #13383.

@Utkarsh
14 years ago

Fixes several validation errors on custom background page.

@Utkarsh
14 years ago

Fixes validation errors on custom header page.

#12 @nacin
14 years ago

(In [14625]) Use valid HTML in the theme editor. span instead of div inside an anchor. props Utkarsh, see #13383.

#13 @nacin
14 years ago

(In [14626]) Ensure validation. props Utkarsh, see #13383.

@Utkarsh
14 years ago

Custom background patch refreshed against r14626

#14 follow-up: @nacin
14 years ago

(In [14628]) Ensure validation. props Utkarsh, see #13383.

#15 in reply to: ↑ 14 @TobiasBg
14 years ago

(In [14628]) Ensure validation.

To avoid the second function call in line 160, we could optimize the code to something like

if ( $bg_color = get_background_color() )
    $background_styles .= "background-color: #{$bg_color};";


instead.

@zeo
14 years ago

Removes unnecessary trailing space in LABEL

@zeo
14 years ago

Standardize FORM id for upload to use lowercase with hyphen. See media.php: <?php echo $type; ?>-form

#16 @zeo
14 years ago

  • Cc zeo@… added

#17 follow-up: @ocean90
14 years ago

  • Cc ocean90@… added

Please pay attention if you change class/id names. You should fix it in the stylesheets too.

Validation erros for custom-header are handled in #13231.

@Utkarsh
14 years ago

Fixes validation errors on ms-sites.php

#18 in reply to: ↑ 17 @zeo
14 years ago

Replying to ocean90:

Please pay attention if you change class/id names. You should fix it in the stylesheets too.

If you're referring to my patch 13383-use-lowercase.diff, uploadForm is not specified in stylesheet. So changing to upload-form doesn't affect anything.

@Utkarsh
14 years ago

Fixes validation errors on ms-users.php

@jshreve
14 years ago

adds missing esc_url calls, fixes many validation errors

#19 @jshreve
14 years ago

I found two missing esc_url calls so far. one on the media buttons (causing errors on dashboard and post screen) and on the set featured image link (errors on post screen). I'm sure that there may be some more.

#20 @nacin
14 years ago

(In [14632]) Valid HTML in ms-users and ms-sites. props Utkarsh, see #13383.

#21 @nacin
14 years ago

(In [14633]) esc_url the URL from get_upload_iframe_src. props jshreve, see #13383.

@TobiasBg
14 years ago

Patch to remove duplicate function call of [14628], see #comment:15

#22 @xibe
14 years ago

press-this.php has a good number of issues, all tied to unescaped JavaScript it seems. A CDATA might help, but I don't if it wouldn't break something else entirely.

#23 follow-up: @nacin
14 years ago

I don't have a problem with 13383-no_duplicate_function_call.patch, but it's hardly the only repetitive function call there. We call get_theme_mod alot.

I'd almost rather keep it. Or, introducing a slew of new variables at the start of the script and then using them is really the only option that will allow us to not make it too spaghetti-like throughout.

Those options are autoloaded, so it's rather trivial.

Re CDATA, see also #11939. Haven't looked at press-this though to see exactly what's going on.

#24 in reply to: ↑ 23 @xibe
14 years ago

Replying to nacin:

Re CDATA, see also #11939. Haven't looked at press-this though to see exactly what's going on.

So since #11939 is now set to Future Release, does it mean we should not look too hard on JS-related validation issues?

@zeo
14 years ago

Media Library: don't wrap div with p tag.

#25 @xibe
14 years ago

A few multisite-related ones. I'm surprised by some, as the files seem to already have gone through proof-validating in this ticket...

wp-admin/network.php:

Line 127, Column 20: required attribute "action" not specified

<form method="post">

Note that here, the network was already activated, so there might not even be a need for a form in the first place...

wp-admin/ms-sites.php

Line 234, Column 30: ID "_wpnonce" already defined

<input type="hidden" id="_wpnonce" name="_wpnonce" value="9983aaea9e" /><in…

Info Line 165, Column 29: ID "_wpnonce" first defined here

<input type="hidden" id="_wpnonce" name="_wpnonce" value="f3ed217f78" /><inp…

wp-admin/ms-sites.php?action=editblog&id=1: cases of duplicated IDs, cases of non-existent IDs

wp-admin/ms-users.php: ID "_wpnonce" already defined

wp-admin/ms-options.php: a dozen of errors, including a seemingly broken table.

@zeo
14 years ago

escape preview link

@Utkarsh
14 years ago

Fixes all validation errors on ms-options.php

#26 @Utkarsh
14 years ago

Added patch for ms-options. Passes validation now.

@zeo
14 years ago

JS: add backlash

#27 @nacin
14 years ago

(In [14655]) Validation and some textarea escaping for ms-options. props Utkarsh, see #13383.

#28 @nacin
14 years ago

(In [14656]) Escape slash in inline JS for SWFUploader. props zeo, see #13383.

#29 @nacin
14 years ago

wp-admin/network.php

Fixing.

wp-admin/ms-sites.php
wp-admin/ms-users.php

In order to fix multiple uses of _wpnonce, we need to make check to see we either only need one of them, or give each one their own name (and change the check_admin_referer checks to reflect that).

wp-admin/ms-sites.php?action=editblog&id=1

Patch welcome. I can imagine that editblog is a mess.

wp-admin/ms-options.php

Fixed in patch by Utkarsh.

#30 @nacin
14 years ago

(In [14657]) Add a form action to network.php. props xibe, see #13383.

@nacin
14 years ago

untested

@Utkarsh
14 years ago

Fixes all validation errors on ms-sites.php

#31 @Utkarsh
14 years ago

Added patch for ms-sites, as reported by xibe.

#32 @nacin
14 years ago

(In [14659]) Validation for network.php. props dremeda, see #13383.

#33 @zeo
14 years ago

nacin,

any reason why you skipped 13383-dont-wrap-div-with-p-tags.diff, and 13383-escape-preview.diff? Both patch tested. 13383-use-lowercase.diff tested but need review and feedback.

#34 @nacin
14 years ago

(In [14672]) Validation for ms-sites.php. props Utkarsh, see #13383.

#35 @nacin
14 years ago

(In [14673]) More validation fixes. Escape some preview=true URLs, also clean up tags in edit-attachment-rows. Slight CSS tweak to match presentation to the formerly invalid HTML. props zeo, see #13383.

@jshreve
14 years ago

fixes some of the multiple nonce errors on multi isite

#36 @jshreve
14 years ago

I didn't see anyone else attempt the nonce stuff. To fix some errors on ms-users and ms-sites I gave the nonces their own names and edited the checks to reflect it (per nacin's comment).

msnonce.diff

@jshreve
14 years ago

Fixes additional errors on the editblog screen in ms-sites.php

#37 @markjaquith
14 years ago

(In [14737]) Misc HTML validation fixes. see #13383. props jshreve

#38 @ocean90
14 years ago

Added patch for documentation link in theme editor.

#39 @ocean90
14 years ago

Added also a patch for new post page. There are also 3 '_ajax_nonce' IDs, should be fixed too. I couldn't find the right nonce checks.

#40 @ocean90
14 years ago

We should hide the Flash box if Javascript is deactivated.
Before: http://grab.by/4xsu | After: http://grab.by/4xsr

#41 @nacin
14 years ago

(In [14930]) Hide Flash uploader and theme/plugin editor documentation feature, if no JS. Also some validation fixes in the theme editor. props ocean90. see #13383.

#42 @nacin
14 years ago

(In [14931]) Validation in wp_dropdown_users and the non-hierarchical taxonomy meta box. props ocean90, see #13383.

#43 @nacin
14 years ago

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

(In [14933]) Separate out multiple _ajax_nonce fields on post forms. Yay, validation. fixes #13383.

Note: See TracTickets for help on using tickets.