WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#22086 closed enhancement (fixed)

HTML5 and cellspacing="0" in admin pages

Reported by: MattyRob Owned by: azaozz
Milestone: 3.9 Priority: normal
Severity: minor Version: 3.4
Component: Administration Keywords: has-patch needs-refresh
Focuses: Cc:

Description

There are several instances where generated table content uses 'cellspacing="0"' in the admin areas of WordPress.

The W3C validator (albeit experimental) states that this parameter should be dropped and CSS used instead for HTML5 compliance.

I think the patch I'm going to attach in a moment will fix these validation issues.

Attachments (6)

22086.patch (7.9 KB) - added by MattyRob 8 years ago.
22086v2.patch (7.8 KB) - added by MattyRob 8 years ago.
22086v3.patch (6.9 KB) - added by MattyRob 8 years ago.
22086v4.patch (6.9 KB) - added by MattyRob 8 years ago.
22086v5.diff (7.6 KB) - added by MattyRob 8 years ago.
22086v6.diff (7.0 KB) - added by MattyRob 7 years ago.

Download all attachments as: .zip

Change History (23)

@MattyRob
8 years ago

#1 @toscho
8 years ago

  • Cc info@… added

padding:0px 0px 0px 0px; should be padding: 0;. Same for other values.

#2 @MattyRob
8 years ago

@toscho - quite right - can't believe I missed that!

#3 @helenyhou
8 years ago

  • Keywords ui-feedback removed

Actually, padding is not equivalent to cellspacing. You want border-collapse and border-spacing. If you look carefully, I think you'd find that removing padding like that results in lots of unwanted side effects. Additionally, you'll want to be sure to only apply the rules to table elements with those selectors, as things like .widefat can be used for all kinds of elements. Also, structural CSS should not go in color files but rather the wp-admin.css sheet.

@MattyRob
8 years ago

#4 @MattyRob
8 years ago

@helenyhou,

I had border-collapse and border-spacing in the patch as well as the padding. I'll take your superior knowledge and remove the padding elements and move the CSS to the wp-admin.css file and submit a revised patch later.

The wp-admin.css file contains this:

/* .widefat - main style for tables */

So, does the styling for this need to be applied only to the HTML tag? Is the class being mis-used or is this documentation incorrect?

#5 follow-up: @MattyRob
8 years ago

Updated patch with CSS moved into wp-admin.css. I just need to check how the CSS is minified. What service is used for this?

@MattyRob
8 years ago

#6 in reply to: ↑ 5 @SergeyBiryukov
8 years ago

Replying to MattyRob:

I just need to check how the CSS is minified. What service is used for this?

There's no need to patch minified files, a post-commit bot takes care of it.

#7 @MattyRob
8 years ago

@SergeyBiryukov - thanks, didn't know that.

v4 patch seems to work better - appearance goes off a little with the first patch - top and bottom of tablets gets cropped.

@MattyRob
8 years ago

#8 @SergeyBiryukov
8 years ago

  • Version changed from trunk to 3.4

#9 @MattyRob
8 years ago

Updated against revision 23455

@MattyRob
8 years ago

#10 @mercime
7 years ago

  • Cc mercijavier@… added

#11 @MattyRob
7 years ago

  • Keywords ui-feedback added; needs-testing 2nd-opinion removed

Updating against version 25671

@MattyRob
7 years ago

#12 @ocean90
7 years ago

  • Keywords needs-refresh added; ui-feedback removed
  • Milestone changed from Awaiting Review to 3.9

Let's refresh this and get it in.

#13 @azaozz
7 years ago

The reason for using cellspacing is IE7. It doesn't understand border-spacing. As support for IE7 is on its way out, perhaps can ignore it in this case too.

#14 @nacin
7 years ago

I'm quite OK with IE7 looking a little off. We currently only care that it is mostly usable and not obviously broken. Pretty is not a goal.

#15 @nacin
7 years ago

I was looking into IE compatibility of these properties. border-spacing is fine in IE8 as long as it is just one value. It looks like border-collapse: separate is the default, though, so is that needed?

#16 @azaozz
7 years ago

Yes, border-collapse: separate is not needed. Also there are many more cellspacing than in the patch.

#17 @azaozz
7 years ago

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

In 27036:

Remove table cellspacing attribute from the admin, part-props MattyRob, fixes #22086.

Note: See TracTickets for help on using tickets.