Download & Extend

ids not unique, should differentiate node and comment id's

Project:Vote Up/Down
Version:5.x-1.x-dev
Component:Code
Category:bug report
Priority:critical
Assigned:Unassigned
Status:closed (won't fix)

Issue Summary

When voting is enabled for nodes and comments,
if a comment has the same id number as the node,
when you vote for that comment,
it looks like your vote gets cast on the node instead of the comment
because the ajax component is only looking for the id to do the update.

But when you refresh the page, you see that your vote is actually cast on the comment correctly. So I think ajax_vote_up_down.js needs some work to differentiate between comments and nodes when doing the instant update.

Comments

#1

Actually, there is a bigger problem here. Both for nodes and for comments, the span id is "vote_up_1" where 1 is the node or comment id. So, in the DOM we have potentially same id elements. This should be fixed by changing the comment ids to something like "vote_up_comment_1". This would fix the problem at its source and there would be no need to change the ajax_vote_up_down.js.

But for the time being, here is a fix to ajax_vote_up_down.js that will prevent the problem by checking the descendants of the element for "node-" or "comments" to differentiate between nodes and comments.

/**
* A Vote DataBase object
*/
Drupal.VDB = function(elt, uri, id) {
  var db = this;
  // By making the span element a property of this object,
  // we get the ability to attach behaviours to that element.
  this.elt = elt;
  this.uri = uri;
  this.id = $(elt).attr('id');
  this.dir1 = this.id.indexOf('vote_up') > -1 ? 'up' : 'down';
  this.dir2 = this.dir1 == 'up' ? 'down' : 'up';
  this.parentFinder = this.uri.indexOf('/comment/') > -1 ? '*[@id=comments]' : '*[@id^=node-]';
  $(elt).click(function() {
    // Ajax GET request for vote data
    $.ajax({
      type: "GET",
      url: db.uri,
      success: function (data) {
        // extract the cid so we can change other elements for the same cid
        var cid = db.id.match(/[0-9]+$/);
        var pid = 'vote_points_' + cid;
        //update the voting arrows
        $(db.parentFinder + ' *#' + db.id + '.vote-' + db.dir1 + '-inact')
          .removeClass('vote-' + db.dir1 + '-inact')
          .addClass('vote-' + db.dir1 + '-act');
        $(db.parentFinder + ' *#' + 'vote_' + db.dir2 + '_' + cid)
          .removeClass('vote-' + db.dir2 + '-act')
          .addClass('vote-' + db.dir2 + '-inact');
        // update the points
        $(db.parentFinder + ' *#' + pid).html(data);
      },
      error: function (xmlhttp) {
        alert('An HTTP error '+ xmlhttp.status +' occured.\n'+ db.uri);
      }
    });
  });
}

So, I have solved the update problem with this little patch, but the multiple elements having the same id problem still needs to be solved. ("vote_up_comment_1", as discussed above)

#2

Status:active» needs review

I just create three patches that fix the error. i also changed the elements id. Please review and let me know if this is the best way to fix, thanks

AttachmentSizeStatusTest resultOperations
ajax_vote_up_down.js-164725.patch1.92 KBIgnored: Check issue status.NoneNone
vote_up_down.module-164725.patch3.41 KBIgnored: Check issue status.NoneNone
vote_up_down.css-164725.patch1.76 KBIgnored: Check issue status.NoneNone

#3

This looks like nice patches! They work well on my local test site.

I attach a updated patch to ajax_vote_up_down.js. It include the changes by awong plus code from the HEAD version to get tooltips working for voting widgets.

AttachmentSizeStatusTest resultOperations
ajax_vote_up_down.js-164725_2.patch2.63 KBIgnored: Check issue status.NoneNone

#4

Status:needs review» closed (won't fix)

Please take a look to the update on the project page, now 5.x is not-really-maintained.

nobody click here