WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#13383 closed task (blessed) (fixed)

Validate the HTML in the admin area

Reported by: 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 6 years ago.
end tag for "tr" omitted in custom-background.php
default-widgets1.diff (1.5 KB) - added by jshreve 6 years ago.
two unclosed 'p' tags
media1p.diff (345 bytes) - added by jshreve 6 years ago.
echoing a </td></tr> instead of adding it to output
13383-theme-editor.diff (3.9 KB) - added by Utkarsh 6 years ago.
Fixes <div> inside <a> in Theme Editor.
13383-custom-background.diff (1.2 KB) - added by Utkarsh 6 years ago.
Fixes several validation errors on custom background page.
13383-custom-header.diff (1.4 KB) - added by Utkarsh 6 years ago.
Fixes validation errors on custom header page.
13383-custom-background.2.diff (1.1 KB) - added by Utkarsh 6 years ago.
Custom background patch refreshed against r14626
13383-remove-trailing-space-in-label.diff (5.4 KB) - added by zeo 6 years ago.
Removes unnecessary trailing space in LABEL
13383-use-lowercase.diff (1.8 KB) - added by zeo 6 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 6 years ago.
Fixes validation errors on ms-sites.php
13383-ms-users.diff (721 bytes) - added by Utkarsh 6 years ago.
Fixes validation errors on ms-users.php
escurl.diff (1.6 KB) - added by jshreve 6 years ago.
adds missing esc_url calls, fixes many validation errors
13383-no_duplicate_function_call.patch (566 bytes) - added by TobiasBg 6 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 6 years ago.
Media Library: don't wrap div with p tag.
13383-escape-preview.diff (1.9 KB) - added by zeo 6 years ago.
escape preview link
13383-ms-options.diff (4.8 KB) - added by Utkarsh 6 years ago.
Fixes all validation errors on ms-options.php
13383-add-backlash.diff (704 bytes) - added by zeo 6 years ago.
JS: add backlash
13383.network.diff (2.3 KB) - added by nacin 6 years ago.
untested
13383-ms-sites.2.diff (1.2 KB) - added by Utkarsh 6 years ago.
Fixes all validation errors on ms-sites.php
msnonce.diff (3.3 KB) - added by jshreve 6 years ago.
fixes some of the multiple nonce errors on multi isite
mssites.diff (1.2 KB) - added by jshreve 6 years ago.
Fixes additional errors on the editblog screen in ms-sites.php
themeeditor.valid.patch (1.3 KB) - added by ocean90 6 years ago.
newpost.valid.patch (1.4 KB) - added by ocean90 6 years ago.
media_upload_for_nojs.patch (1.4 KB) - added by ocean90 6 years ago.

Download all attachments as: .zip

Change History (67)

#1 @Viper007Bond
6 years ago

  • Keywords needs-patch added
  • Version set to 3.0

@jshreve
6 years ago

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

#2 @jshreve
6 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
6 years ago

two unclosed 'p' tags

@jshreve
6 years ago

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

#3 follow-up: @jshreve
6 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
6 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
6 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
6 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
6 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
6 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
6 years ago

  • Cc dre@… added

@Utkarsh
6 years ago

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

#10 @Utkarsh
6 years ago

  • Cc admin@… added

dremeda, just submitted a patch for that :)

#11 @nacin
6 years ago

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

@Utkarsh
6 years ago

Fixes several validation errors on custom background page.

@Utkarsh
6 years ago

Fixes validation errors on custom header page.

#12 @nacin
6 years ago

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

#13 @nacin
6 years ago

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

@Utkarsh
6 years ago

Custom background patch refreshed against r14626

#14 follow-up: @nacin
6 years ago

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

#15 in reply to: ↑ 14 @TobiasBg
6 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
6 years ago

Removes unnecessary trailing space in LABEL

@zeo
6 years ago

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

#16 @zeo
6 years ago

  • Cc zeo@… added

#17 follow-up: @ocean90
6 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
6 years ago

Fixes validation errors on ms-sites.php

#18 in reply to: ↑ 17 @zeo
6 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
6 years ago

Fixes validation errors on ms-users.php

@jshreve
6 years ago

adds missing esc_url calls, fixes many validation errors

#19 @jshreve
6 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
6 years ago

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

#21 @nacin
6 years ago

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

@TobiasBg
6 years ago

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

#22 @xibe
6 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
6 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
6 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
6 years ago

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

#25 @xibe
6 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
6 years ago

escape preview link

@Utkarsh
6 years ago

Fixes all validation errors on ms-options.php

#26 @Utkarsh
6 years ago

Added patch for ms-options. Passes validation now.

@zeo
6 years ago

JS: add backlash

#27 @nacin
6 years ago

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

#28 @nacin
6 years ago

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

#29 @nacin
6 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
6 years ago

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

@nacin
6 years ago

untested

@Utkarsh
6 years ago

Fixes all validation errors on ms-sites.php

#31 @Utkarsh
6 years ago

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

#32 @nacin
6 years ago

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

#33 @zeo
6 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
6 years ago

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

#35 @nacin
6 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
6 years ago

fixes some of the multiple nonce errors on multi isite

#36 @jshreve
6 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
6 years ago

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

#37 @markjaquith
6 years ago

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

#38 @ocean90
6 years ago

Added patch for documentation link in theme editor.

#39 @ocean90
6 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
6 years ago

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

#41 @nacin
6 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
6 years ago

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

#43 @nacin
6 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.