Hi,

I've just installed the extra_voting_forms module, configured it, and almost worked ;-) I say that because when I write a story, and (logged as another user) I vote it up (or down), it works, and updates the number of votes for that story. The same happens when I vote a related comment. BUT, I think there is a bug in some situations:

+ User A posts a story X
+ User B writes a comment about that story X
+ User C writes another comment for the same story X

Now if you read the HTML code for the X page, you'll see that the same oid is used for the original story X's vote counter
and the oldest comment (that posted by user B, the last comment of the page)

By example :

So, if another user D votes for the comment written by B, he will see that the updated counter will be for the X story and not for the comment written by B. It's a transient bug, because if the user reloads the page, both the story and the comments will be correctly printed. The module works ok for other comments (just fails for the oldest one).

Note: I've done all the tests in a local virtual host. If you can't reproduce the bug I can install it in a public accesible host so you can test it more easily. TIA.

Juanan

CommentFileSizeAuthor
#6 extra_voting_forms-karma_score-268858-6.patch3.51 KBsikjoy
#5 extra_voting_forms-karma_score-268858-5.patch1.58 KBsikjoy

Comments

juanan’s picture

juanan commented

In response to myself: further investigation revealed that it's not a problem about repeated oid numbers, because there is a otype differente attribute for nodes and comments. Nevertheless, the bug behaviour still applies, that is, if I vote up for the oldest comment, the vote is aggregated (added) to the node's karma_score attribute (and not to othe comment's one)

mercmobily’s picture

mercmobily commented

Hi,

Oh dear, this is really bad.
Can you please confirm that this is a Javascript issue?
That is, the Javascript is updating the vote box in the wrong spot after the vote; however, the vote is assigned to the right node/comment?

Also, now that you know that repeated OIDs are not an issue, are you able to post a slightly simplify way of reproducing this?

Bye!

Merc.

sikjoy’s picture

sikjoy commented
Assigned: Unassigned » sikjoy
sikjoy’s picture

sikjoy commented

Hi juanan:

Can you tell me which of the forms you are using for node karma and which form are you using for comment karma?

Thanks,

sicjoy

sikjoy’s picture

sikjoy commented
Version: 5.x-1.14 » 5.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new extra_voting_forms-karma_score-268858-5.patch1.58 KB

Hi:

I was able to fix this, but there's more work to be done.

The problem was a result of the fact that the forms for the node and the oldest comment each had the same id. In my case, I was using form 3 for both nodes and comments, it was <form id="x1">.

I modified the Javascript so that it checks the otype of the voting form then inserts the appropriate ancestor (.node or .comment) into the selector string before updating the karma score.

Extra Voting Forms still breaks one of CSS's basic rules: no two tags can have the same id on the same page.

I'm submitting this patch until a better solution can be found. Perhaps something as simple as inserting the otype into the form id will suffice.

--sicjoy

sikjoy’s picture

sikjoy commented
StatusFileSize
new extra_voting_forms-karma_score-268858-6.patch3.51 KB

Alrighty! This should be the proper way to fix this bug.

Would someone please test this patch thoroughly, esp. any of you Drigg users, as my changes may have far reaching repercussions. We really need to get simpletest tests written for this module. Any takers?

Cheers,

sicjoy

mercmobily’s picture

mercmobily commented

Hi Sicjoy,

Yeah i am in Nicaragua...
:-)
First of all, thanks for working on this. One note: I remember working really hard so that no javascript depended on any template layout. Is this still true, with your patch? I am asking because you mentioned that you are apprending the right thing to .node or .comment, and right now I am not quite sure if that would apply to a .tpl.php file or to hte output generated by the form. Remember, a theme could potentially have a different name form 'comments' or 'nodes'... I liked the fact that drigg had indipendence from the therme.

Other than that... back to the heat :-) Sorry I can't help much, but I don't have access to the codebase right now!

Bye,

Merc,

sikjoy’s picture

sikjoy commented

Hi Merc:

I agree that you should strive to have form layout independence. In my second patch I dropped use of the (.node or .comment) ancestor requirement as that information is now encoded in the karma_score span id. That's where the ambiguity was that caused this bug in the first place. Under certain situations the karma_score span for a comment and a node could both have the same id.

--sicjoy

mercmobily’s picture

mercmobily commented

Hi,

That sounds like the perfect way to go about it!

Merc.

sikjoy’s picture

sikjoy commented
Status: Needs review » Fixed
Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.