MediaWiki r54153 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r54152‎ | r54153 (on ViewVC)‎ | r54154 >
Date:16:44, 1 August 2009
Author:shinjiman
Status:resolved (Comments)
Tags:
Comment:
* (bug 12110) Split the rights for editing users' CSS/JS subpage from "editusercssjs" into "editusercss" and edituserjs" respectively.
rebuild message file for all languages
Modified paths:

Follow-up revisions

Rev.Commit summaryAuthorDate
r54194Added back 'editusercssjs' right for regression and backward compatibility, p...shinjiman08:31, 2 August 2009
r54247Added back 'right-editusercssjs' message for backward compatibility, for r54194....shinjiman00:11, 3 August 2009
r56492Fix logic error from r54153. By negating each individual isAllowed check, the...mrzman04:27, 17 September 2009

Comments

#Comment by Raymond (talk | contribs)   17:16, 1 August 2009

Hmmmm what about b/c for installations like:

$wgGroupPermissions['sysop']['editusercssjs']    = false;
$wgGroupPermissions['foo']['editusercssjs']    = true;

With this commit it breaks the security model.

#Comment by Shinjiman (talk | contribs)   22:05, 1 August 2009

Perhaps adding back the hook for 'editusercssjs' for compat setting?

#Comment by Shinjiman (talk | contribs)   08:31, 2 August 2009

Added back 'editusercssjs' regression test on r54194.

#Comment by Simetrical (talk | contribs)   17:43, 2 August 2009

Why did you duplicate the existing messages? The new messages are wrong, they imply that both rights work for both CSS and JS. Like I'm guessing

 'right-editusercss'           => 'Editar las páginas de CSS y JS de otros usuarios',
 'right-edituserjs'            => 'Editar las páginas de CSS y JS de otros usuarios',

should be more like

 'right-editusercss'           => 'Editar las páginas de CSS de otros usuarios',
 'right-edituserjs'            => 'Editar las páginas de JS de otros usuarios',

You should probably just not provide any message, so translators can provide new messages that are correct.

Generally speaking, why don't you just introduce the two new rights and not remove the old right or anything relating to it at all? You need to keep the old right for backward compatibility, so you should keep the old messages too (otherwise ListGroupRights won't work on wikis using the old right), etc. There's no reason for us to not have to support the old right indefinitely.

#Comment by Shinjiman (talk | contribs)   00:12, 3 August 2009

Restored 'right-editusercssjs', updated message 'right-editusercss' and 'right-edituserjs' for the language used on r54247.

Status & tagging log

  • 23:25, 19 August 2009 Brion VIBBER (talk | contribs) changed the status of r54153 [removed: fixme added: resolved]
  • 17:16, 1 August 2009 Raymond (talk | contribs) changed the status of r54153 [removed: new added: fixme]