WordPress.org

Make WordPress Core

Opened 4 years ago

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

Download all attachments as: .zip

Change History (67)

comment:1 Viper007Bond4 years ago

  • Keywords needs-patch added
  • Version set to 3.0

jshreve4 years ago

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

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

jshreve4 years ago

two unclosed 'p' tags

jshreve4 years ago

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

comment:3 follow-up: jshreve4 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.

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

comment:5 nacin4 years ago

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

comment:6 in reply to: ↑ 3 Viper007Bond4 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. :)

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

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

comment:9 dremeda4 years ago

  • Cc dre@… added

Utkarsh4 years ago

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

comment:10 Utkarsh4 years ago

  • Cc admin@… added

dremeda, just submitted a patch for that :)

comment:11 nacin4 years ago

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

Utkarsh4 years ago

Fixes several validation errors on custom background page.

Utkarsh4 years ago

Fixes validation errors on custom header page.

comment:12 nacin4 years ago

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

comment:13 nacin4 years ago

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

Utkarsh4 years ago

Custom background patch refreshed against r14626

comment:14 follow-up: nacin4 years ago

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

comment:15 in reply to: ↑ 14 TobiasBg4 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.

zeo4 years ago

Removes unnecessary trailing space in LABEL

zeo4 years ago

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

comment:16 zeo4 years ago

  • Cc zeo@… added

comment:17 follow-up: ocean904 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.

Utkarsh4 years ago

Fixes validation errors on ms-sites.php

comment:18 in reply to: ↑ 17 zeo4 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.

Utkarsh4 years ago

Fixes validation errors on ms-users.php

jshreve4 years ago

adds missing esc_url calls, fixes many validation errors

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

comment:20 nacin4 years ago

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

comment:21 nacin4 years ago

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

TobiasBg4 years ago

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

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

comment:23 follow-up: nacin4 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.

comment:24 in reply to: ↑ 23 xibe4 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?

zeo4 years ago

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

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

zeo4 years ago

escape preview link

Utkarsh4 years ago

Fixes all validation errors on ms-options.php

comment:26 Utkarsh4 years ago

Added patch for ms-options. Passes validation now.

zeo4 years ago

JS: add backlash

comment:27 nacin4 years ago

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

comment:28 nacin4 years ago

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

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

comment:30 nacin4 years ago

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

nacin4 years ago

untested

Utkarsh4 years ago

Fixes all validation errors on ms-sites.php

comment:31 Utkarsh4 years ago

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

comment:32 nacin4 years ago

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

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

comment:34 nacin4 years ago

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

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

jshreve4 years ago

fixes some of the multiple nonce errors on multi isite

comment:36 jshreve4 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

jshreve4 years ago

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

comment:37 markjaquith4 years ago

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

ocean904 years ago

comment:38 ocean904 years ago

Added patch for documentation link in theme editor.

ocean904 years ago

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

comment:40 ocean904 years ago

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

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

comment:42 nacin4 years ago

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

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