Activity reply buttons have duplicate IDs

[boonebgorges]

See some discussion on #3253

'Reply' buttons on activity items belonging to the same root activity item have the same ID, 'acomment-reply-x' (where x is the activity id). Duplicate IDs make for invalid markup.

This problem does not have an easy fix, because the button ID is used by the javascript to determine which root activity should be set as the item_id of the newly created activity item. See around line 305-308, line 327, and lines 344-350 of bp-default/_inc/global.js

A couple different strategies to fix:

• Add an additional hidden input field to the activity reply markup that contains the ID the root activity item, and modify the JS to use that that value
• In the JS, get the root activity ID by crawling up the tree using jQuery .parents() or .find() or something like that. (I did a preliminary test of this and it seemed to be quite slow, so ymmv.)
• Change the button ID so that it contains the root activity ID *in addition to* the replied-to activity ID. Thus something like acomment-reply-x-y, where x is the root activity id, and y is the immediate parent id. Then modify the JS to parse this correctly.

IMO this is not urgent because it has always been this way, and it's not affecting the way the page works. Any takers?

[Dennissmolek]

I'll take this.
There are a few solutions to this.
Firstly there is changing the ID field in the comments.php file created in the last patch(#3253)
id="acomment-reply-<?php bp_activity_id() ?>-from-<?php bp_activity_comment_id() ?>"
This would add which object it was coming from.
The JS is set up so that ANY object clicked triggers a multitude of events. In this case it reads the ID of the object clicked and pulls apart its ID. The ID is currently acomment-reply-x. When it does so it splits the string by each dash, so we get an array(acomment, reply, x). Where X is the activity ID that we are commenting on.
The script then pulls item [2] from the array, which in this case is the activity ID.

By adding -from-y (where Y is the ID of the comment the reply button is being clicked) the array now looks like (acomment, reply, x, from, y) This does not break the JS and currently works on my testing site. Without pulling apart the js just to make the ID's more semantic I think this is the fastest and best solution.

Note:

I think we are at a point where we could start thinking about HTML5 and what we could do with it. By using the data-x="" attribute we could simply write data-comment="" I'm writing other stuff and I have to think that hiding data in the ID of a element will become a faux pas when we can use data- much like tables were/are in the CSS era.
link to what I'm talking about: ​http://ejohn.org/blog/html-5-data-attributes/
Two things to note on this issue.

1. If we change the code to use HTML5(which is the future) it wont validate on a older doctype(PS it already doesnt)
2. The JS doesnt care, we can simply use .attr() and adjust the code slightly. Older browsers wont have issues with the code, they will simply ignore the attributes, but the JS will still run.

What use does this have you ask? with this reply button not much, besides limiting the use of insane ID's and odd code. The ID could be "acomment-reply-1" which is semantic and valid.

Just an idea I noticed when working on this.. Like I said, the quick fix is adding the little bit of code. The rest is just an idea I'm proposing..

[boonebgorges]

Good point about the JS remaining the same with your fix. That should be a fine fix for right now. Please go ahead and post a patch.

I don't mind introducing HTML5 into the markup where it will degrade gracefully in crappy browsers. But we shouldn't make changes to the JavaScript that will not be backward compatible. Let's stick with class and id for the moment, at least as far as jQuery selectors are concerned.

[Dennissmolek]

@Boone I agree. I talked to Paul and he is opposed to using HMTL5 right now. I think once themes and plugins for WP start using it more and more we will see both WP and BP moving forward with html5.

Here is my patch with my basic fix. Tested on my site and working.

[Dennissmolek]

adds -from-y where y is the comment replying from

[boonebgorges]

This looks pretty good as a fix for the moment. I'll have a look later today and commit it, unless anyone has an objection to the strategy.

[boonebgorges]

(In [4522]) Modifies activity reply comment form IDs so as to avoid duplicates on a single page. Fixes #3279. Props Dennissmolek