r113311 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r113310‎ | r113311 | r113312 >
Date:22:34, 7 March 2012
Author:emsmith
Status:ok (Comments)
Tags:
Comment:
bug 34090 - fixing the username bugs - apparently using the data- stuff with jquery makes .data() not work right, so went back to own custom attributes and pushing them into the javascript method when populating the name/timestamp
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js
@@ -469,14 +469,14 @@
470470 *
471471 * @param $row element the feedback row
472472 */
473 - $.articleFeedbackv5special.markHidden = function ( $row ) {
 473+ $.articleFeedbackv5special.markHidden = function ( $row, $hide_user, $hide_timestamp ) {
474474 if ( $row.data( 'hidden' ) ) {
475475 $.articleFeedbackv5special.unmarkHidden( $row );
476476 }
477477 $row.addClass( 'articleFeedbackv5-feedback-hidden' )
478478 .data( 'hidden', true );
479479 $( '<span class="articleFeedbackv5-feedback-hidden-marker"></span>' )
480 - .text( mw.msg( 'articlefeedbackv5-hidden', $row.data('hide-user'), $row.data('hide-timestamp')) )
 480+ .text( mw.msg( 'articlefeedbackv5-hidden', $hide_user, $hide_timestamp) )
481481 .insertBefore( $row.find( '.articleFeedbackv5-comment-wrap h3' ) );
482482 $.articleFeedbackv5special.maskPost( $row );
483483 };
@@ -516,14 +516,14 @@
517517 *
518518 * @param $row element the feedback row
519519 */
520 - $.articleFeedbackv5special.markDeleted = function ( $row ) {
 520+ $.articleFeedbackv5special.markDeleted = function ( $row , $oversight_user, $oversight_timestamp) {
521521 if ( $row.data( 'deleted' ) ) {
522522 $.articleFeedbackv5special.unmarkDeleted();
523523 }
524524 $row.addClass( 'articleFeedbackv5-feedback-deleted' )
525525 .data( 'deleted', true );
526526 var $marker = $( '<span class="articleFeedbackv5-feedback-deleted-marker"></span>' )
527 - .text( mw.msg( 'articlefeedbackv5-deleted', $row.data('oversight-user'), $row.data('oversight-timestamp') ) )
 527+ .text( mw.msg( 'articlefeedbackv5-deleted', $oversight_user, $oversight_timestamp ) )
528528 .insertBefore( $row.find( '.articleFeedbackv5-comment-wrap h3' ) );
529529 $.articleFeedbackv5special.maskPost( $row );
530530 };
@@ -752,10 +752,10 @@
753753 }
754754 }
755755 if ( $( this ).hasClass( 'articleFeedbackv5-feedback-hidden' ) ) {
756 - $.articleFeedbackv5special.markHidden( $( this ) );
 756+ $.articleFeedbackv5special.markHidden( $( this ), $( this ).attr('hide-user'), $( this ).attr('hide-timestamp'));
757757 }
758758 if ( $( this ).hasClass( 'articleFeedbackv5-feedback-deleted' ) ) {
759 - $.articleFeedbackv5special.markDeleted( $( this ) );
 759+ $.articleFeedbackv5special.markDeleted( $( this ), $( this ).attr('oversight-user'), $( this ).attr('oversight-timestamp'));
760760 }
761761 } );
762762 $( '#articleFeedbackv5-feedback-count-total' ).text( data['articlefeedbackv5-view-feedback'].count );
@@ -974,7 +974,9 @@
975975 $link.removeClass( 'abusive' );
976976 }
977977 if ( data['articlefeedbackv5-flag-feedback']['abuse-hidden'] ) {
978 - $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ) );
 978+ $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ),
 979+ data['articlefeedbackv5-flag-feedback']['hide-user'],
 980+ data['articlefeedbackv5-flag-feedback']['hide-timestamp']);
979981 }
980982 $link.attr( 'id', 'articleFeedbackv5-unabuse-link-' + id )
981983 .removeClass( 'articleFeedbackv5-abuse-link' )
@@ -1010,7 +1012,9 @@
10111013 $link.removeClass( 'abusive' );
10121014 }
10131015 if ( data['articlefeedbackv5-flag-feedback']['abuse-hidden'] ) {
1014 - $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ) );
 1016+ $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ),
 1017+ data['articlefeedbackv5-flag-feedback']['hide-user'],
 1018+ data['articlefeedbackv5-flag-feedback']['hide-timestamp']);
10151019 }
10161020 $link.attr( 'id', 'articleFeedbackv5-abuse-link-' + id )
10171021 .removeClass( 'articleFeedbackv5-unabuse-link' )
@@ -1034,10 +1038,9 @@
10351039 .removeClass( 'articleFeedbackv5-hide-link' )
10361040 .addClass( 'articleFeedbackv5-show-link' );
10371041
1038 - $link.data( 'hide-user', data['articlefeedbackv5-flag-feedback']['hide-user']);
1039 - $link.data( 'hide-timestamp', data['articlefeedbackv5-flag-feedback']['hide-timestamp']);
1040 -
1041 - $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ) );
 1042+ $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ),
 1043+ data['articlefeedbackv5-flag-feedback']['hide-user'],
 1044+ data['articlefeedbackv5-flag-feedback']['hide-timestamp']);
10421045 $.articleFeedbackv5special.setActivityFlag( id, 'hide', true );
10431046 }
10441047 },
@@ -1075,8 +1078,19 @@
10761079 .text( mw.msg( 'articlefeedbackv5-form-unoversight' ) )
10771080 .removeClass( 'articleFeedbackv5-requestoversight-link' )
10781081 .addClass( 'articleFeedbackv5-unrequestoversight-link');
 1082+
10791083 if ( data['articlefeedbackv5-flag-feedback']['autohidden'] ) {
1080 - $.articleFeedbackv5special.markHidden( $link.closest( '.articleFeedbackv5-feedback' ) );
 1084+ var $new_link = $( '#articleFeedbackv5-hide-link-' + id )
 1085+ .attr( 'action', 'show' )
 1086+ .attr( 'id', 'articleFeedbackv5-show-link-' + id )
 1087+ .text( mw.msg( 'articlefeedbackv5-form-unhide' ) )
 1088+ .removeClass( 'articleFeedbackv5-hide-link' )
 1089+ .addClass( 'articleFeedbackv5-show-link' );
 1090+
 1091+ $.articleFeedbackv5special.markHidden( $new_link.closest( '.articleFeedbackv5-feedback' ),
 1092+ data['articlefeedbackv5-flag-feedback']['hide-user'],
 1093+ data['articlefeedbackv5-flag-feedback']['hide-timestamp']);
 1094+ $.articleFeedbackv5special.setActivityFlag( id, 'hide', true );
10811095 }
10821096 }
10831097 },
@@ -1113,10 +1127,9 @@
11141128 .removeClass( 'articleFeedbackv5-oversight-link' )
11151129 .addClass( 'articleFeedbackv5-unoversight-link' );
11161130
1117 - $link.data( 'oversight-user', data['articlefeedbackv5-flag-feedback']['oversight-user']);
1118 - $link.data( 'oversight-timestamp', data['articlefeedbackv5-flag-feedback']['oversight-timestamp']);
1119 -
1120 - $.articleFeedbackv5special.markDeleted( $link.closest( '.articleFeedbackv5-feedback' ) );
 1131+ $.articleFeedbackv5special.markDeleted( $link.closest( '.articleFeedbackv5-feedback' ) ,
 1132+ data['articlefeedbackv5-flag-feedback']['oversight-user'],
 1133+ data['articlefeedbackv5-flag-feedback']['oversight-timestamp']);
11211134 $.articleFeedbackv5special.setActivityFlag( id, 'delete', true );
11221135 }
11231136 },
Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php
@@ -508,9 +508,9 @@
509509 $name = 'Article Feedback V5';
510510 }
511511
512 - $attributes['data-hide-user'] = $name;
 512+ $attributes['hide-user'] = $name;
513513 if ($record[0]->af_hide_timestamp > 0) {
514 - $attributes['data-hide-timestamp'] = wfTimestamp( TS_RFC2822, $record[0]->af_hide_timestamp );
 514+ $attributes['hide-timestamp'] = wfTimestamp( TS_RFC2822, $record[0]->af_hide_timestamp );
515515 }
516516 }
517517 if ( $record[0]->af_is_deleted ) {
@@ -525,9 +525,9 @@
526526 $name = 'Article Feedback V5';
527527 }
528528
529 - $attributes['data-oversight-user'] = $name;
 529+ $attributes['oversight-user'] = $name;
530530 if ($record[0]->af_oversight_timestamp > 0) {
531 - $attributes['data-oversight-timestamp'] = wfTimestamp( TS_RFC2822, $record[0]->af_oversight_timestamp );
 531+ $attributes['oversight-timestamp'] = wfTimestamp( TS_RFC2822, $record[0]->af_oversight_timestamp );
532532 }
533533 }
534534

Follow-up revisions

RevisionCommit summaryAuthorDate
r113317bug 34090 - js and css voodoo to make the element with the red lines appear a...emsmith23:01, 7 March 2012
r113370bug 34090 - make different titles for masking appear if it's been hidden or o...emsmith16:43, 8 March 2012
r113371bug 34090 - fixes for oversighter view for hide/oversight panelsemsmith17:51, 8 March 2012
r113383bug 34090 - not entirely necessary, but keeps $2 from showing up in hiders pa...emsmith19:25, 8 March 2012
r113384bug 34090 - fix for double red line issues when hiding an oversighted post ha...emsmith19:28, 8 March 2012
r113390bug 34090 - adding translation for "automatic hider" useremsmith19:44, 8 March 2012
r113392bug 34090 - followup to r113287 - adjusted localization documentationemsmith20:03, 8 March 2012
r113393bug 34090 - followup to r113370 - adjusted localization documentationemsmith20:05, 8 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r111211bug 34090 - Added a log namespace and log types for the Article Feedback v5 e...emsmith22:53, 10 February 2012
r111472bug 34090 - Added an additional column in the main feedback table to keep a c...emsmith19:14, 14 February 2012
r111474bug 34090 - remove a todo - take the note from the flag submission and save i...emsmith19:57, 14 February 2012
r111552bug 34090 - split activity notes into their own config for maximum length (th...emsmith16:07, 15 February 2012
r111557bug 34090 - changed structure of the link classes and ids per the following f...emsmith16:47, 15 February 2012
r111570bug 34090 - request oversight is now a counter - so you can request/unrequest...emsmith19:53, 15 February 2012
r111573bug 34090 - removed the "header" portion of the html generation from the acti...emsmith20:18, 15 February 2012
r111645bug 34090 - updated the filter count update script to get requested oversight...emsmith15:45, 16 February 2012
r112038bug 34090 - added additional counts for filters including unhidden, undeleted...emsmith20:28, 21 February 2012
r112039bug 34090 - fixed issue noted : if $feedback is false return nothing - not th...emsmith20:50, 21 February 2012
r112041bug 34090 - kill the -1 issue by never letting it get below 0emsmith21:08, 21 February 2012
r112115bug 34090 - no code changes, just fixing/adding keyword svn propertiesemsmith16:31, 22 February 2012
r112119bug 34090 - cast the naughty column so it can be signed, the greatest still k...emsmith16:57, 22 February 2012
r112142bug 34090 - toggle for atomic un/helpful changing (and elimination of an extr...emsmith20:37, 22 February 2012
r112147bug 34090 - note to self, watch the copy and paste errors...emsmith21:01, 22 February 2012
r112149bug 34090 - no upper limit, only lower limit - we can't get any worse then 0emsmith21:21, 22 February 2012
r112154bug 34090 - quick and dirty helper script to get missing documentation keysemsmith21:57, 22 February 2012
r112156bug 34090 - only send the activity header if continue < 1, make the more div ...emsmith22:14, 22 February 2012
r112161bug 34090 - fix limit and use old continue to determine if we do the headeremsmith23:15, 22 February 2012
r112218bug 34090 - make sure the name are right for hidden/unhidden logging (argh)emsmith16:44, 23 February 2012
r112225bug 34090 - unhidden and unoversight logic adjustmentsemsmith18:06, 23 February 2012
r112228bug 34090 - fix filter - needsoversight are always autohiddenemsmith19:16, 23 February 2012
r112230bug 34090 - let's try this again - needsoversight and declined will have hidd...emsmith19:38, 23 February 2012
r112232bug 34090 - hide and show shouldn't fiddle with oversight counts or declined ...emsmith19:48, 23 February 2012
r112599bug 34090 - follow up to r111211 - rename things to make them "less confusin...emsmith14:43, 28 February 2012
r112603bug 34090 - Add the logging of automated hide/show errors by the "Article Fee...emsmith15:21, 28 February 2012
r112604bug 34090 - can't believe there were no permissions checks in this - only del...emsmith15:38, 28 February 2012
r112610bug 34090 - change all sql updates to use escaping except for explicitly comm...emsmith16:22, 28 February 2012
r112611bug 34090 - take out implicit show and add returning user that hid/oversighte...emsmith17:02, 28 February 2012
r112627bug 34090 - follow up to r112599 - change to use getText since it's going int...emsmith18:53, 28 February 2012
r112825bug 34090 - insert custom attributes (sigh) for the user and formatted timest...emsmith18:14, 1 March 2012
r112830bug 34090 - follow up to r111474 - use truncate for choppingemsmith19:42, 1 March 2012
r113052bug 34090 - remaining backend feature in requirements, oversight email genera...emsmith18:04, 5 March 2012
r113062bug 34090 - follow up to r110520 1. change index 2. default of null for conti...emsmith18:54, 5 March 2012
r113073bug 34090 - follow up to r111472 part 1 - change to use getDbKey, check for b...emsmith19:48, 5 March 2012
r113082bug 34090 - follow up to r111471 - changed to use text()emsmith20:42, 5 March 2012
r113083bug 34090 - follow up to rr111472 part 2 - only use log_id for orderingemsmith20:48, 5 March 2012
r113104bug 34090 - follow up to rr111472 part 3 - totally redo the continue function...emsmith23:10, 5 March 2012
r113159bug 34090 - follow up to rr111472 part 4 and follow up to r111596 (same issue...emsmith17:37, 6 March 2012
r113160bug 34090 - follow up to rr111472 part 5 plural and number format action countemsmith17:48, 6 March 2012
r113161bug 34090 - follow up to rr111472 part 6 last of lego messagesemsmith17:53, 6 March 2012
r113163bug 34090 - two additional configuration settings (help url and admin user ur...emsmith18:02, 6 March 2012
r113193bug 34090 - followup to r113104 - only sort by timestamp (sorting by log id w...emsmith22:56, 6 March 2012
r113228bug 34090 - followup to r113160emsmith14:05, 7 March 2012
r113247bug 34090 - db issue, remove one of the sorts from the query, use the ids arr...emsmith16:52, 7 March 2012
r113269bug 34090 - followup to r113247emsmith19:01, 7 March 2012
r113273bug 34090 - add javascript level hiding on request oversight IF autohidden is...emsmith19:22, 7 March 2012
r113287bug 34090 - usernames and formatted timestamps into red lines for hidden/over...emsmith20:22, 7 March 2012

Comments

#Comment by Catrope (talk | contribs)   18:17, 8 March 2012

Hmm, what broke here? data-foo is supposed to Just Work, I'd be interested to hear what kinds of issues you ran into.

#Comment by Elizabeth M Smith (talk | contribs)   18:21, 8 March 2012

data-foo works - kind of

If the attribute is present in the html it is properly added to data and I can get it out and change it as expected (that was pretty cool)

however, when I then tried to add data('user-hide') to elements that did not have it present at html load time, I could not

Not sure if this is "spec" or if it's a jquery bug (seriously think it's a jquery bug)

If I added the attr with an empty value into the html at load time, it worked fine... But there are four possible data attributes per feedback row so that would mean putting 4 empty data-blah attributes on every row displayed so I could change the users/timestamps as necessary - the overhead in that is insane

#Comment by Catrope (talk | contribs)   18:23, 8 March 2012

That's weird, setting yet-unheard-of data is supposed to work even in the presence of other data- attributes. Maybe it's a bug in the specific version of jQuery we're using or something.

#Comment by Elizabeth M Smith (talk | contribs)   18:25, 8 March 2012

I highly suspect it's a JQuery bug, but for now the workaround functions fine ("That's weird" seems to be the singular reaction to everyone I show that bug to ;)

Status & tagging log