r97369 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97368‎ | r97369 | r97370 >
Date:04:56, 17 September 2011
Author:neilk
Status:ok (Comments)
Tags:
Comment:
reverted r96988
Modified paths:
  • /trunk/extensions/UploadWizard/UploadWizard.config.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizard.i18n.php (modified) (history)
  • /trunk/extensions/UploadWizard/includes/UploadWizardCampaign.php (modified) (history)
  • /trunk/extensions/UploadWizard/includes/specials/SpecialUploadCampaign.php (modified) (history)
  • /trunk/extensions/UploadWizard/includes/specials/SpecialUploadWizard.php (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizardDeed.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizardLicenseInput.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/UploadWizard.config.php
@@ -68,13 +68,6 @@
6969 // The maximum length of the id field.
7070 'idFieldMaxLength' => 25,
7171
72 - // Wikitext for a custom own-work license.
73 - // Leave empty for no such license.
74 - 'wikitextLicense' => '',
75 -
76 - // Templates to add when choosing the custom in wikitext defined license.
77 - 'wikitextLicenseTemplates' => array(),
78 -
7972 // 'licenses' is a list of licenses you could possibly use elsewhere, for instance in
8073 // licensesOwnWork or licensesThirdParty.
8174 // It just describes what licenses go with what wikitext, and how to display them in
Index: trunk/extensions/UploadWizard/includes/specials/SpecialUploadCampaign.php
@@ -94,12 +94,7 @@
9595
9696 // show _ALL_ the licenses!
9797 $standardConfig = UploadWizardConfig::getConfig();
98 - $licences = array_merge(
99 - $standardConfig['licenses'],
100 - array( 'wikitextLicense' => array( 'msg' => 'mwe-upwiz-campaign-customLicense' ) )
101 - );
102 -
103 - foreach ( $licences as $key => $license ) {
 98+ foreach ( $standardConfig['licenses'] as $key => $license ) {
10499 $configFields['licensesOwnWork']['options'][ wfMsg( $license['msg'] ) ] = $key;
105100 $configFields['defaultOwnWorkLicence']['options'][ wfMsg( $license['msg'] ) ] = $key;
106101 }
Index: trunk/extensions/UploadWizard/includes/specials/SpecialUploadWizard.php
@@ -168,15 +168,6 @@
169169
170170 $config['thanksLabel'] = $this->getPageContent( $config['thanksLabelPage'], true );
171171
172 - if ( in_array( 'wikitextLicense', $config['licensesOwnWork']['licenses'] ) ) {
173 - $config['wikitextLicense'] = str_replace( '$1', $this->getLang()->getCode(), $config['wikitextLicense'] );
174 -
175 - $config['licenses']['wikitextLicense'] = array(
176 - 'html' => $this->getOutput()->parse( $config['wikitextLicense'], false ),
177 - 'templates' => $config['wikitextLicenseTemplates'],
178 - );
179 - }
180 -
181172 $this->getOutput()->addScript(
182173 Skin::makeVariablesScript(
183174 array(
Index: trunk/extensions/UploadWizard/includes/UploadWizardCampaign.php
@@ -198,12 +198,6 @@
199199 'options' => array(),
200200 'default' => $globalConfig['licensesOwnWork']['defaults'][0]
201201 ),
202 - 'wikitextLicense' => array(
203 - 'type' => 'text'
204 - ),
205 - 'wikitextLicenseTemplates' => array(
206 - 'type' => 'text'
207 - ),
208202 'defaultCategories' => array(
209203 'type' => 'text'
210204 ),
@@ -219,6 +213,13 @@
220214 ),
221215 );
222216
 217+ foreach ( $globalConfig['licensesOwnWork']['licenses'] as $license ) {
 218+ $licenceMsg = wfMsg( $globalConfig['licenses'][$license]['msg'] );
 219+ $config['licensesOwnWork']['options'][$licenceMsg] = $license;
 220+ }
 221+
 222+ $config['defaultOwnWorkLicence']['options'] = $config['licensesOwnWork']['options'];
 223+
223224 return $config;
224225 }
225226
Index: trunk/extensions/UploadWizard/UploadWizard.i18n.php
@@ -322,7 +322,6 @@
323323 'uploadcampaign-text' => 'You are modifying an Upload Wizard campaign.',
324324 'mwe-upwiz-campaign-name' => 'Campaign name:',
325325 'mwe-upwiz-campaign-enabled' => 'Campaign enabled',
326 - 'mwe-upwiz-campaign-customLicense' => 'Custom license',
327326 'mwe-upwiz-campaign-conf-skipTutorial' => 'Skip the licensing tutorial',
328327 'mwe-upwiz-campaign-conf-autoCategories' => 'Categories to add the files to automatically and silently (pipe separated):',
329328 'mwe-upwiz-campaign-conf-defaultCategories' => 'Default categories to list in the describe tab (pipe separated):',
@@ -342,8 +341,6 @@
343342 'mwe-upwiz-campaign-conf-headerLabelPage' => 'Page containing text to display above the UploadWizard interface. $1 is replaced with the language code:',
344343 'mwe-upwiz-campaign-conf-thanksLabelPage' => 'Page containing text to display on top of the "Use" page. $1 is replaced with the language code:',
345344 'mwe-upwiz-campaign-conf-idFieldMaxLength' => 'Maximum length of the text in the ID field',
346 - 'mwe-upwiz-campaign-conf-wikitextLicense' => 'Wikitext for a custom own-work license. Empty for no such custom license. $1 is replaced with the language code:',
347 - 'mwe-upwiz-campaign-conf-wikitextLicenseTemplates' => 'Templates to add when the user chooses the custom license (pipe separated):',
348345
349346 // Coolcats
350347 'mw-coolcats-confirm-new-title' => 'Confirm new category',
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizardDeed.js
@@ -55,14 +55,16 @@
5656 .attr( { name: "author" } )
5757 .addClass( 'mwe-upwiz-sign' );
5858
59 - _this.showCustomDiv = mw.UploadWizard.config.licensesOwnWork.licenses.length > 1;
 59+ var ownWork = mw.UploadWizard.config.licensesOwnWork;
 60+ var licenseIsNotDefault = ( ownWork.licenses.length === 1 && ownWork.licenses[0] !== ownWork.defaults[0] );
 61+ _this.showCustomDiv = ownWork.licenses.length > 1 || licenseIsNotDefault;
6062
6163 if ( _this.showCustomDiv ) {
6264 var licenseInputDiv = $j( '<div class="mwe-upwiz-deed-license"></div>' );
6365
6466 _this.licenseInput = new mw.UploadWizardLicenseInput(
6567 licenseInputDiv,
66 - undefined,
 68+ undefined,
6769 mw.UploadWizard.config.licensesOwnWork,
6870 _this.uploadCount
6971 );
@@ -89,9 +91,8 @@
9092 return this.licenseInput.getWikiText();
9193 }
9294 else {
93 - var defLicensename = mw.UploadWizard.config.licensesOwnWork.defaults[0];
9495 return '{{' + mw.UploadWizard.config.licensesOwnWork.filterTemplate
95 - + '|' + mw.UploadWizard.config.licenses[defLicensename]['templates'].join( '' ) + '}}';
 96+ + '|' + mw.UploadWizard.config.licensesOwnWork.defaults[0] + '}}';
9697 }
9798 },
9899
@@ -118,29 +119,15 @@
119120 _this.$form = $j( '<form />' );
120121
121122 _this.$authorInput2 = $j( '<input type="text" />' ).attr( { name: "author2" } ).addClass( 'mwe-upwiz-sign' );
122 -
123 - var $assertNote = $j( '<p class="mwe-small-print"></p>' );
124 -
125 - var defLicense = mw.UploadWizard.config.licensesOwnWork.defaults[0];
126 - defLicense = mw.UploadWizard.config.licenses[defLicense];
127 -
128 - if ( mw.isDefined( defLicense.html ) ) {
129 - $assertNote.text( gM( 'mwe-upwiz-source-ownwork-assert-note', '' ) )
130 - .append( defLicense.html )
131 - }
132 - else {
133 - $assertNote.msg(
134 - 'mwe-upwiz-source-ownwork-assert-note',
135 - gM( defLicense.msg )
136 - )
137 - }
138 -
139123 var $standardDiv = $j( '<div />' ).append(
140124 $j( '<label for="author2" generated="true" class="mwe-validator-error" style="display:block;" />' ),
141125 $j( '<p></p>' ).msg( 'mwe-upwiz-source-ownwork-assert',
142126 uploadCount,
143127 _this.$authorInput2 ),
144 - $assertNote
 128+ $j( '<p class="mwe-small-print"></p>' ).msg(
 129+ 'mwe-upwiz-source-ownwork-assert-note',
 130+ gM( 'mwe-upwiz-license-' + mw.UploadWizard.config.licensesOwnWork.defaults[0] )
 131+ )
145132 );
146133
147134 var $crossfader = $j( '<div />' ).append( $standardDiv );
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizardLicenseInput.js
@@ -83,28 +83,16 @@
8484 $input.data( 'licenseName', name );
8585 _this.inputs.push( $input );
8686
 87+ var messageKey = mw.isDefined( license.props['msg'] ) ? license.props.msg : '[missing msg for ' + license.name + ']';
8788 var $icons = $j( '<span></span>' );
8889 if ( mw.isDefined( license.props['icons'] ) ) {
8990 $j.each( license.props.icons, function( i, icon ) {
9091 $icons.append( $j( '<span></span>' ).addClass( 'mwe-upwiz-license-icon mwe-upwiz-' + icon + '-icon' ) );
9192 } );
9293 }
93 -
94 - var $label = $j( '<label />' ).attr( { 'for': id } ).append( $icons );
95 -
96 - if ( mw.isDefined( license.props['html'] ) ) {
97 - $label.html( license.props['html'] );
98 - }
99 - else if ( mw.isDefined( license.props['msg'] ) ) {
100 - $label.msg( license.props['msg'], _this.count );
101 - }
102 - else {
103 - $label.text( '[missing msg for ' + license.name + ']' );
104 - }
105 -
10694 $el.append(
10795 $input,
108 - $label,
 96+ $j( '<label />' ).attr( { 'for': id } ).msg( messageKey, _this.count ).append( $icons ),
10997 $j( '<br/>' )
11098 // XXX help?
11199 );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r96988fixed bug 29249; also fixed issues caused by using the license name where it'...jeroendedauw18:54, 13 September 2011

Comments

#Comment by Jeroen De Dauw (talk | contribs)   15:24, 17 September 2011

I'm not convinced removing this feature makes sense to do. It might not be the solution needed for the bug I was trying to fix, but I think it can definitely be useful, and is not in anyone's way. People can create campaigns with own-work licenses not in the list, without having to rage someone into adding it to the codebase and then waiting for deploy.

#Comment by Jeroen De Dauw (talk | contribs)   15:13, 19 September 2011
  • Poke*
#Comment by NeilK (talk | contribs)   17:41, 19 September 2011

Pulling wikitext out of configuration seems (IMO) a bad idea. Templates are stored in the wiki db. It's bad enough that we do have configuration related to the names of templates. If I could revisit this I'd probably do what the community seems to have wanted, and obtain such config from wiki pages (perhaps with defaults in code).

But if you feel strongly about this then commit it and we'll deal with it like any contributed code... at the moment though it wasn't what we really wanted

#Comment by Jeroen De Dauw (talk | contribs)   17:53, 19 September 2011

> and obtain such config from wiki pages (perhaps with defaults in code).

Well, sure, this makes sense for config in general, but not for campaign specific config, which is what this setting was.

> at the moment though it wasn't what we really wanted

It's been created already, so even though this might not be needed for this bug, it can be useful later on.

So yeah, I'd prefer reverting the revert, over doing a partial revert with only the bugfixes my original commit also contains.

Status & tagging log