r80796 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r80795‎ | r80796 | r80797 >
Date:09:03, 23 January 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
<s> is deprecated, change for <del>

Add definition of $matches before use in preg call
Modified paths:
  • /trunk/extensions/CodeReview/CodeReview.i18n.php (modified) (history)
  • /trunk/extensions/CodeReview/ui/CodeRevisionView.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CodeReview/CodeReview.i18n.php
@@ -262,7 +262,7 @@
263263 'code-signoff-submit' => 'Submit button text.
264264
265265 A "sign-off" is a concept in code review that means that the person doing the sign-off has approved the involved code changes.',
266 - 'code-signoff-strike' => 'Submit button that, when clicked, will cause the selected sign-offs to be struck. Struck sign-offs are visible but displayed <s>with a line through them</s>.',
 266+ 'code-signoff-strike' => 'Submit button that, when clicked, will cause the selected sign-offs to be struck. Struck sign-offs are visible but displayed <del>with a line through them</del>.',
267267 'code-signoff-signoff' => 'Label text which is followed by a checkbox for each sign-off state and a submit button.',
268268 'code-signoff-flag-inspected' => 'Type of sign-off indicating the code has been looked at.',
269269 'code-signoff-flag-tested' => 'Type of sign-off indicating the code has been tested.',
Index: trunk/extensions/CodeReview/ui/CodeRevisionView.php
@@ -241,6 +241,7 @@
242242 // Uses messages 'code-rev-modified-a', 'code-rev-modified-r', 'code-rev-modified-d', 'code-rev-modified-m'
243243 $desc = wfMsgHtml( 'code-rev-modified-' . strtolower( $action ) );
244244 // Find any ' (from x)' from rename comment in the path.
 245+ $matches = array();
245246 preg_match( '/ \([^\)]+\)$/', $path, $matches );
246247 $from = isset( $matches[0] ) ? $matches[0] : '';
247248 // Remove ' (from x)' from rename comment in the path.

Comments

#Comment by P858snake (talk | contribs)   00:56, 24 January 2011

I mentioned this to reedy in IRC last night, but are we sure <del> is semantically correct here? it's designed to show content which is deleted from a page, by the looks of <s>, we might just be wanting a span with styling here.

#Comment by Reedy (talk | contribs)   01:12, 24 January 2011

http://www.w3schools.com/tags/tag_strike.asp said to "use <del> instead"

Span with the right style would work though also

#Comment by P858snake (talk | contribs)   01:18, 24 January 2011

W3Schools is known to have a "few" issues with it's examples and documentation: http://w3fools.com/

It was probably bad example for me to use it last night.

#Comment by Dantman (talk | contribs)   02:20, 24 January 2011

Obligatory spec link: http://www.whatwg.org/specs/web-apps/current-work/multipage/edits.html#the-del-element

I'll give that the example we're giving in the i18n message is not actual deletion, but it "is" an example. In that light I believe that anything correct in the context of what we're referring to in the message should be valid in the context of the example, and should be preferred over something that looks like the markup used but isn't.

So looking at the context we're referring to. We're dealing with sign-offs which are being stricken, in essence, they are being deleted. Now while the spec says "The del element represents a removal from the document." I'd hope they didn't intend it to mean that in html5 is only valid when actually editing a html document and marking up what was removed. Because with other things like <article>, <section>, <nav>, etc... it feels out of place for not to be valid as markup for deletion in general. (in fact I'd suggest making a ml comment on getting that improved)

#Comment by Hashar (talk | contribs)   21:40, 12 February 2011

I am marking this as ok. The & drama remembers me the <trong> & drama we had years ago. It is just unproductive.

#Comment by Hashar (talk | contribs)   21:40, 12 February 2011

and here I trigger a codereview bug :)

#Comment by Reedy (talk | contribs)   23:27, 12 February 2011

& drama remembers me the <trong> &

#Comment by Reedy (talk | contribs)   23:28, 12 February 2011

lolol. Fixed

Status & tagging log