locale.inc uses deprecated elements and attributes [#27668]

Comment	File	Size	Author
#8	xhtml_fixes_0.zip	2.3 KB	wulff
#6	xhtml_fixes.zip	2.3 KB	wulff
#3	drupal.css_0.patch	424 bytes	wulff

#2	locale.inc.patch	3.05 KB	wulff

	locale.inc.html.patch	3.08 KB	wulff

Comment #1

Junyor CreditAttribution: Junyor commented 28 July 2005 at 10:16

I'd prefer to see the styling done in drupal.css, rather than inline. +1 for the fix, -1 for the implementation.

Log in or register to post comments

Comment #2

wulff CreditAttribution: wulff commented 28 July 2005 at 11:09

File	Size
locale.inc.patch	3.05 KB

You are absolutely right. This patch fixes locale.inc.

Log in or register to post comments

Comment #3

wulff CreditAttribution: wulff commented 28 July 2005 at 11:10

File	Size
drupal.css_0.patch	424 bytes

...and this one adds the styles to drupal.css.

Log in or register to post comments

Comment #4

breyten CreditAttribution: breyten commented 28 July 2005 at 11:17

Better, but where did the CSS go? ;) Furthermore, I like semantic markup better, ie. <em class="locale-untranslated"> instead of using spans.

Log in or register to post comments

Comment #5

Robrecht Jacques CreditAttribution: Robrecht Jacques commented 28 July 2005 at 11:21

+1 for the CSS/class solution.

maybe the class "locale-nowrap" could become a more general class "nowrap" as this could be used in more locations, eg in drupal:

modules/statistics.module:        array('data' => format_date($log->timestamp, 'small'), 'nowrap' => 'nowrap'),
modules/statistics.module:        array('data' => format_date($log->timestamp, 'small'), 'nowrap' => 'nowrap'),
modules/statistics.module:      array('data' => format_date($log->timestamp, 'small'), 'nowrap' => 'nowrap'),

"locale-untranslated" is locale.module dependent, so that one is ok.

Log in or register to post comments

Comment #6

wulff CreditAttribution: wulff commented 28 July 2005 at 12:13

File	Size
xhtml_fixes.zip	2.3 KB

breyten: Agreed.

Updated patches with fixes for statistics.module attached.

Log in or register to post comments

Comment #7

Robrecht Jacques CreditAttribution: Robrecht Jacques commented 28 July 2005 at 12:21

You have put 'style' => 'nowrap' instead of 'class' => 'nowrap' on all locations. Small thinko I suppose.
If that's fixed => +1 from me.

Log in or register to post comments

Comment #8

wulff CreditAttribution: wulff commented 28 July 2005 at 12:31

File	Size
xhtml_fixes_0.zip	2.3 KB

A thinko indeed. Fixed fixes attached.

Log in or register to post comments

Comment #9

drumm

he/him

NY, US

CreditAttribution: drumm commented 28 July 2005 at 17:53

I'm not sure if this is the right tag for this, but there is a del tag which might be a better alternative to strike. http://www.w3.org/TR/html401/struct/text.html#h-9.4

You can make patchfiles which include multiple files instead of making a zip file which adds an extra step for reviewers.
cvs diff -uF^f misc/drupal.css includes/locale.inc.

Log in or register to post comments

Comment #10

Robrecht Jacques CreditAttribution: Robrecht Jacques commented 28 July 2005 at 18:58

"del" is not the right tag here. It is not because the default style for "del" is strike-through that it is "correct" to use it if you replacing something with "style=strikethrough". "del" (and "ins") is for document changes, and only for that. Here the strike-through doesn't mean that.

So a "span", or maybe "em" (you put emphasis on the fact that it is untranslated?) is better.