WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#8971 closed defect (bug) (fixed)

Invalid value for @cols in default theme comments.php textarea element?

Reported by: sampablokuper Owned by:
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.7
Component: Themes Keywords: has-patch commit early
Focuses: Cc:

Description

In the default theme, comments.php has a textarea element thus:

<textarea name="comment" id="comment" cols="100%" rows="10" tabindex="4"></textarea>

If I'm not mistaken, the cols attribute should be a number (not a percentage), according to, e.g., http://www.w3.org/TR/html401/interact/forms.html#h-17.7 and http://www.velocityreviews.com/forums/t153587-html-forms-and-liquid-designs.html and

Attachments (1)

8971.diff (682 bytes) - added by vladimir_kolesnikov 6 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 @Denis-de-Bernardy6 years ago

  • Keywords needs-patch added
  • Milestone changed from 2.8 to Future Release

@vladimir_kolesnikov6 years ago

comment:2 @vladimir_kolesnikov6 years ago

  • Cc vladimir@… added
  • Keywords has-patch added; needs-patch removed

comment:3 @vladimir_kolesnikov6 years ago

  • Milestone changed from Future Release to 2.8

comment:4 @Denis-de-Bernardy6 years ago

  • Keywords commit added

comment:5 follow-up: @hakre6 years ago

Looks very ok to me, in current trunk there is a CSS width of 100% in style.css therefore this should be pretty alright.

Is there a specific argument for having cols="58 " instead of let's say 71?

comment:6 in reply to: ↑ 5 @vladimir_kolesnikov6 years ago

Replying to hakre:

Is there a specific argument for having cols="58 " instead of let's say 71?

Yes. I removed "width: 100%" in FireBug and strted playing with the value of cols. 58 looks almost the same as width: 100%.

comment:7 @Denis-de-Bernardy6 years ago

  • Keywords early added
  • Milestone changed from 2.8 to 2.9

can wait until 2.8.1

comment:8 @hakre6 years ago

  • Milestone changed from 2.9 to 2.8

But must not.

comment:9 follow-up: @Denis-de-Bernardy6 years ago

  • Milestone changed from 2.8 to 2.9

the patch *might* break a child theme here and there that overrides the font

comment:10 in reply to: ↑ 9 @vladimir_kolesnikov6 years ago

Replying to Denis-de-Bernardy:

the patch *might* break a child theme here and there that overrides the font

Are you sure?

  1. Those who remove width: 100% from CSS I guess know what they do
  2. cols="100%" is invalid XHTML and is either ignored by the user agent or interpretes is as "100" (which, without CSS, is not good at all).
  3. At any rate, width: 100% in CSS overrules the width calculated from the cols attribute.

comment:11 @Denis-de-Bernardy6 years ago

width=100% occasionally wrecks havok in IE6 (or was that FF2?).

either way, it's just validation stuff. it's been invalid for years, it's not a regression, it won't hurt to be invalid for another month.

comment:12 @hakre6 years ago

Well that is not the appropriate argumentation to deal with defects. You can use that argument for any bug, especially those that are in for a long time. That is pretty contra-productive with that much legacy codebase that is in WordPress.

comment:13 @Denis-de-Bernardy6 years ago

still applies clean

comment:14 @hakre6 years ago

one week to left for "another month" :D.

comment:15 follow-up: @westi6 years ago

  • Cc westi added

Can't quite see why we have cols here at all.

Why can't we just rely on the css to set the width?

comment:16 in reply to: ↑ 15 @vladimir_kolesnikov6 years ago

Replying to westi:

Why can't we just rely on the css to set the width?

Because cols and rows are required attributes of textarea in (X)HTML.

comment:17 @azaozz5 years ago

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

Fixed in [11867]

Note: See TracTickets for help on using tickets.