r105193 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105192‎ | r105193 | r105194 >
Date:17:26, 5 December 2011
Author:rsterbin
Status:ok
Tags:
Comment:
Tweaks and bug fixes for jquery plugin, as per Roan's review:
- Use mw.util.wikiScript( 'api' ) to get the URL for api.php
- Namespaced find-feedback id
- Terms page link always comes from wgArticleFeedbackv5TermsPage
- Terms page doesn't need wikiGetLink() anymore
- Added var before t in loops (leaked to the global scope)
- Removed unused attribute "original-title" on trash icon
- Closed html:msg tag
- Removed old template lines from bucket 5's buildForm()
- Removed anontoken parameter from viewratings api call
- Wrapped console.log() calls in object checks
- Replaced someone's wonky mw.msg() calls with html:msg tags in the edit cta template
- Fixed places where .html() was used for message contents with .text()
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
——@@ -109,7 +109,7 @@
110110 * Use the mediawiki util resource's config method to find the correct url to
111111 * call for all ajax requests.
112112 */
113 - $.articleFeedbackv5.apiUrl = mw.config.get( 'wgScriptPath' ) + '/api.php';
 113+ $.articleFeedbackv5.apiUrl = mw.util.wikiScript( 'api' );
114114
115115 /**
116116 * Is this an anonymous user?
——@@ -240,7 +240,7 @@
241241 <div class="clear"></div>\
242242 </div>\
243243 <div class="articleFeedbackv5-comment">\
244 - <textarea id="find-feedback" class="feedback-text" name="comment"></textarea>\
 244+ <textarea id="articleFeedbackv5-find-feedback" class="feedback-text" name="comment"></textarea>\
245245 </div>\
246246 <div class="articleFeedbackv5-disclosure">\
247247 <!-- <p class="articlefeedbackv5-shared-on-feedback"></p> -->\
——@@ -294,7 +294,7 @@
295295 .html( $.articleFeedbackv5.buildLink(
296296 'articlefeedbackv5-transparency-terms',
297297 {
298 - href: 'http://wikimediafoundation.org/wiki/Feedback_privacy_statement',
 298+ href: mw.config.get( 'wgArticleFeedbackv5TermsPage' ),
299299 text: 'articlefeedbackv5-transparency-terms-linktext',
300300 target: '_blank'
301301 } ) );
——@@ -360,9 +360,6 @@
361361 .keyup ( function () {
362362 if( $( this ).val().length > 0 ) {
363363 $.articleFeedbackv5.enableSubmission( true );
364 -// } else {
365 -// $.articleFeedbackv5.enableSubmission( false );
366 -
367364 }
368365 } )
369366 .blur( function () {
——@@ -482,7 +479,7 @@
483480 <div class="clear"></div>\
484481 </div>\
485482 <div class="articleFeedbackv5-comment">\
486 - <textarea id="find-feedback" class="feedback-text" name="comment"></textarea>\
 483+ <textarea id="articleFeedbackv5-find-feedback" class="feedback-text" name="comment"></textarea>\
487484 </div>\
488485 <div class="articleFeedbackv5-disclosure">\
489486 <!-- <p class="articlefeedbackv5-shared-on-feedback"></p> -->\
——@@ -574,7 +571,7 @@
575572 .html( $.articleFeedbackv5.buildLink(
576573 'articlefeedbackv5-transparency-terms',
577574 {
578 - href: mw.util.wikiGetlink( mw.config.get( 'wgArticleFeedbackv5TermsPage' ) ),
 575+ href: mw.config.get( 'wgArticleFeedbackv5TermsPage' ),
579576 text: 'articlefeedbackv5-transparency-terms-linktext',
580577 target: '_blank'
581578 } ) );
——@@ -618,9 +615,6 @@
619616 .keyup ( function () {
620617 if( $( this ).val().length > 0 ) {
621618 $.articleFeedbackv5.enableSubmission( true );
622 -// } else {
623 -// $.articleFeedbackv5.enableSubmission( false );
624 -
625619 }
626620 } )
627621 .blur( function () {
——@@ -679,7 +673,7 @@
680674 if ( $c.val() == '') {
681675 empty = true;
682676 } else {
683 - for ( t in $.articleFeedbackv5.currentBucket().commentDefault ) {
 677+ for ( var t in $.articleFeedbackv5.currentBucket().commentDefault ) {
684678 if ( $c.val() == $.articleFeedbackv5.currentBucket().commentDefault[t] ) {
685679 empty = true;
686680 }
——@@ -709,7 +703,7 @@
710704 var data = {};
711705 data.tag = $.articleFeedbackv5.find( '.articleFeedbackv5-tags input[checked]' ).val();
712706 data.comment = $.articleFeedbackv5.find( '.articleFeedbackv5-comment textarea' ).val();
713 - for ( t in $.articleFeedbackv5.currentBucket().commentDefault ) {
 707+ for ( var t in $.articleFeedbackv5.currentBucket().commentDefault ) {
714708 if ( data.comment == $.articleFeedbackv5.currentBucket().commentDefault[t] ) {
715709 data.comment = '';
716710 }
——@@ -773,7 +767,7 @@
774768 <div class="articleFeedbackv5-rating-label" rel="3"></div>\
775769 <div class="articleFeedbackv5-rating-label" rel="4"></div>\
776770 <div class="articleFeedbackv5-rating-label" rel="5"></div>\
777 - <div class="articleFeedbackv5-rating-clear" original-title="Remove this rating"></div>\
 771+ <div class="articleFeedbackv5-rating-clear"></div>\
778772 </div>\
779773 </div>\
780774 <div style="clear:both;"></div>\
——@@ -784,7 +778,7 @@
785779 <div class="clear"></div>\
786780 </div>\
787781 <div class="articleFeedbackv5-comment">\
788 - <textarea id="find-feedback" class="feedback-text" name="comment"></textarea>\
 782+ <textarea id="articleFeedbackv5-find-feedback" class="feedback-text" name="comment"></textarea>\
789783 </div>\
790784 <div class="articleFeedbackv5-disclosure">\
791785 <!-- <p class="articlefeedbackv5-shared-on-feedback"></p> -->\
——@@ -853,7 +847,7 @@
854848 .html( $.articleFeedbackv5.buildLink(
855849 'articlefeedbackv5-transparency-terms',
856850 {
857 - href: mw.util.wikiGetlink( mw.config.get( 'wgArticleFeedbackv5TermsPage' ) ),
 851+ href: mw.config.get( 'wgArticleFeedbackv5TermsPage' ),
858852 text: 'articlefeedbackv5-transparency-terms-linktext',
859853 target: '_blank'
860854 } ) );
——@@ -947,9 +941,6 @@
948942 .keyup ( function () {
949943 if( $( this ).val().length > 0 ) {
950944 $.articleFeedbackv5.enableSubmission( true );
951 -// } else {
952 -// $.articleFeedbackv5.enableSubmission( false );
953 -
954945 }
955946 } )
956947 .blur( function () {
——@@ -1061,7 +1052,7 @@
10621053 block: '\
10631054 <div>\
10641055 <div class="form-row articleFeedbackv5-bucket4-toggle">\
1065 - <p class="sub-header"><strong><html:msg key="bucket4-subhead"></strong></p>\
 1056+ <p class="sub-header"><strong><html:msg key="bucket4-subhead" /></strong></p>\
10661057 <p class="instructions-left"><html:msg key="bucket4-teaser-line1" /><br />\
10671058 <html:msg key="bucket4-teaser-line2" /></p>\
10681059 </div>\
——@@ -1281,10 +1272,6 @@
12821273 */
12831274 buildForm: function () {
12841275
1285 - // A single rating block
1286 - var rating_tpl = '\
1287 - ';
1288 -
12891276 // Start up the block to return
12901277 var $block = $( $.articleFeedbackv5.currentBucket().templates.block );
12911278
——@@ -1318,7 +1305,7 @@
13191306 .html( $.articleFeedbackv5.buildLink(
13201307 'articlefeedbackv5-bucket5-form-panel-helpimprove-note',
13211308 {
1322 - href: mw.util.wikiGetlink( mw.config.get( 'wgArticleFeedbackv5TermsPage' ) ), // TODO: Make this work
 1309+ href: mw.config.get( 'wgArticleFeedbackv5TermsPage' ), // TODO: Make this work
13231310 text: 'articlefeedbackv5-bucket5-form-panel-helpimprove-privacy'
13241311 }
13251312 ) );
——@@ -1622,7 +1609,6 @@
16231610 'format': 'json',
16241611 'list': 'articlefeedbackv5-view-ratings',
16251612 'afpageid': $.articleFeedbackv5.pageId,
1626 - 'afanontoken': $.articleFeedbackv5.userId,
16271613 'maxage': 0,
16281614 'smaxage': mw.config.get( 'wgArticleFeedbackv5SMaxage' )
16291615 },
——@@ -1637,7 +1623,7 @@
16381624 var msg = mw.msg ( 'articlefeedbackv5-error-response' );
16391625 if ( 'error' in data && 'info' in data.error ) {
16401626 msg = data.error.info;
1641 - } else {
 1627+ } else if ( typeof console != 'undefined' ) {
16421628 console.log(data);
16431629 }
16441630 $.articleFeedbackv5.markShowstopperError( msg );
——@@ -1793,13 +1779,14 @@
17941780 <div class="articleFeedbackv5-confirmation-panel">\
17951781 <div class="articleFeedbackv5-panel-leftContent">\
17961782 <div class="articleFeedbackv5-confirmation-text">\
1797 - <span class="articleFeedbackv5-confirmation-thanks">' + mw.msg( "articlefeedbackv5-cta1-thanks" ) + '</span> <span class="articleFeedbackv5-confirmation-follow-up">' + mw.msg( "articlefeedbackv5-cta1-confirmation-followup" ) + '</span>\
 1783+ <span class="articleFeedbackv5-confirmation-thanks"><html:msg key="cta1-thanks" /></span>\
 1784+ <span class="articleFeedbackv5-confirmation-follow-up"><html:msg key="cta1-confirmation-followup" /></span>\
17981785 </div>\
1799 - <h3 class="articleFeedbackv5-confirmation-title">' + mw.msg( "articlefeedbackv5-cta1-confirmation-title" ) + '</h3>\
1800 - <p class="articleFeedbackv5-confirmation-wikipediaWorks">' + mw.msg( "articlefeedbackv5-cta1-confirmation-call" ) + '</p>\
1801 - <p class="articleFeedbackv5-confirmation-learnHow"><a target="_blank" href="#">' + mw.msg("articlefeedbackv5-cta1-learn-how") + ' &raquo;</a></p>\
 1786+ <h3 class="articleFeedbackv5-confirmation-title"><html:msg key="cta1-confirmation-title" /></h3>\
 1787+ <p class="articleFeedbackv5-confirmation-wikipediaWorks"><html:msg key="cta1-confirmation-call" /></p>\
 1788+ <p class="articleFeedbackv5-confirmation-learnHow"><a target="_blank" href="#"><html:msg key="cta1-learn-how" /> &raquo;</a></p>\
18021789 </div>\
1803 - <a href="&amp;action=edit" class="articleFeedbackv5-edit-cta-link"><span class="ui-button-text">' + mw.msg( "articlefeedbackv5-cta1-edit-linktext" ) + '</span></a>\
 1790+ <a href="&amp;action=edit" class="articleFeedbackv5-edit-cta-link"><span class="ui-button-text"><html:msg key="cta1-edit-linktext" /></span></a>\
18041791 <div class="clear"></div>\
18051792 </div>\
18061793 '
——@@ -1928,7 +1915,7 @@
19291916 $.articleFeedbackv5.bucketId,
19301917 { 'expires': cfg.expires, 'path': '/' }
19311918 );
1932 - if ( $.articleFeedbackv5.debug ) {
 1919+ if ( $.articleFeedbackv5.debug && typeof console != 'undefined' ) {
19331920 console.log( 'Using bucket #' + $.articleFeedbackv5.bucketId );
19341921 }
19351922 };
——@@ -2034,8 +2021,8 @@
20352022 bucket.enableSubmission( state );
20362023 }
20372024 $.articleFeedbackv5.submissionEnabled = state;
2038 - $( '#articleFeedbackv5-submit-bttn span' ).html( mw.msg( 'articlefeedbackv5-bucket1-form-submit' ) );
2039 - $( '#articleFeedbackv5-submit-bttn5 span' ).html( mw.msg( 'articlefeedbackv5-bucket5-form-panel-submit' ) );
 2025+ $( '#articleFeedbackv5-submit-bttn span' ).text( mw.msg( 'articlefeedbackv5-bucket1-form-submit' ) );
 2026+ $( '#articleFeedbackv5-submit-bttn5 span' ).text( mw.msg( 'articlefeedbackv5-bucket5-form-panel-submit' ) );
20402027 };
20412028
20422029 // }}}
——@@ -2262,6 +2249,7 @@
22632250 if ( 'bindEvents' in cta ) {
22642251 cta.bindEvents( $block );
22652252 }
 2253+ $block.localize( { 'prefix': 'articlefeedbackv5-' } );
22662254 if ( 'getTitle' in cta ) {
22672255 if ( $.articleFeedbackv5.inDialog ) {
22682256 $.articleFeedbackv5.$dialog.dialog( 'option', 'title', cta.getTitle() );
——@@ -2315,22 +2303,22 @@
23162304 * @param string message the message to display, if in dev
23172305 */
23182306 $.articleFeedbackv5.markShowstopperError = function ( message ) {
2319 - console.log( message );
2320 - var msg_html = mw.msg( 'articlefeedbackv5-error' );
2321 - if ($.articleFeedbackv5.debug && message) {
2322 - msg_html = message.replace( "\n", '<br />' );
 2307+ if ( typeof console != 'undefined' ) {
 2308+ console.log( message );
23232309 }
2324 - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-error-message' ).html( msg_html );
2325 - var veil = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-error' );
2326 - var box = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-panel' );
 2310+ var $err = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-error-message' );
 2311+ $err.text( $.articleFeedbackv5.debug && message ? message : mw.msg( 'articlefeedbackv5-error' ) );
 2312+ $err.html( $err.html().replace( "\n", '<br />' ) );
 2313+ var $veil = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-error' );
 2314+ var $box = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-buffer' );
23272315 // TODO: Make this smarter -- on ubuntu/ff at least, using the
23282316 // offset puts it about 100px down from where it should be;
23292317 // this math corrects for it, but will most likely be wrong on
23302318 // other browsers
2331 - veil.css('top', box.find('.articleFeedbackv5-ui').offset().top / 2 + 10);
2332 - veil.css('width', box.width());
2333 - veil.css('height', box.height());
2334 - veil.show();
 2319+ $veil.css('top', $box.find('.articleFeedbackv5-ui').offset().top / 2 + 10);
 2320+ $veil.css('width', $box.width());
 2321+ $veil.css('height', $box.height());
 2322+ $veil.show();
23352323 };
23362324
23372325 // }}}
——@@ -2368,7 +2356,9 @@
23692357 }
23702358 }
23712359 if ( $.articleFeedbackv5.debug ) {
2372 - console.log( errors );
 2360+ if ( typeof console != 'undefined' ) {
 2361+ console.log( errors );
 2362+ }
23732363 }
23742364 if ( 'markFormErrors' in $.articleFeedbackv5.currentBucket() ) {
23752365 $.articleFeedbackv5.currentBucket().markFormErrors( errors );

Status & tagging log