Problem

The attribute rel is used to group links in a gallery. It is done allways, even with special rel values like "nofollow".

Proposed resolution

Check that rel attribute value is not a well known value intended for a specific use.
In colorbox_node.js ($.fn.colorboxNodeGroup, about line 100) could be done with something like this:

var rel = $(this).attr('rel');
var knownRels=new Array('alternate', 'author', 'bookmark', 'help', 'license', 'next', 'nofollow', 'noreferrer', 'prefetch', 'prev', 'search', 'tag');
if ($('a[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")').length > 1 && $.inArray(rel, knownRels)==-1) {
CommentFileSizeAuthor
#15 colorbox_node-better-gallery-implementation-2120319-15.patch3.84 KBAndre-B
#11 colorbox_module-rel_nofollow-2120319-11.patch862 bytesAndre-B
#10 colorbox_module-rel_nofollow-2120319-10.patch855 bytesAndre-B
#9 colorbox_module-rel_nofollow-2120319-9.patch1.71 KBAndre-B
#6 Screenshot 2013-11-14 19.00.35.png24.12 KBAndre-B
#3 colorbox_module-rel_nofollow-2120319-3.patch2.31 KBmiguel.svq
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

iLLin’s picture

iLLin CreditAttribution: iLLin commented

I would like it more if this was an admin setting instead of hard coded so the user can add w/e they want. We can start with defaults but it would be better if it was editable.

miguel.svq’s picture

miguel.svq CreditAttribution: miguel.svq commented

I do agree.

Maybe better this in .js (L 119):

if ($('a[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")').length > 1 && $.inArray(rel, Drupal.settings['colorbox_node']['known_rels'] )==-1 ) {

In .module, (L 17):

  $colorbox_node_known_rels= array(
   'known_rels' => preg_split("/[,\s]+/", variable_get('colorbox_node_known_rels', 'alternate, author, bookmark, help, license, next, nofollow, noreferrer, prefetch, prev, search, tag'), -1, PREG_SPLIT_NO_EMPTY),
  );
  drupal_add_js(array('colorbox_node' => $colorbox_node_known_rels), 'setting');

...and in colorbox_node_form_alter (about L 55)

      $form['colorbox_node']['colorbox_node_known_rels'] = array(
        '#type' => 'textfield',
        '#title' => t('Don\'t make galleries with those rel attribute values'),
        '#description' => t('Enter a comma-separated list of values. The links with those rel attribute values will NOT make galleries.'),
        '#default_value' => variable_get('colorbox_node_known_rels', 'alternate, author, bookmark, help, license, next, nofollow, noreferrer, prefetch, prev, search, tag'),
      );
miguel.svq’s picture

miguel.svq CreditAttribution: miguel.svq commented
Status: Active » Needs review
FileSize
colorbox_module-rel_nofollow-2120319-3.patch2.31 KB

I'm unsure about the patch format and naming, hope it's right.

iLLin’s picture

iLLin CreditAttribution: iLLin commented

K, I will work this in when I get a chance. Thanks!

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented

I have additional issues with the gallery function in the latest "stable" colorbox version:

for no reason I get a gallery feature for a colorbox link, that does not contain any rel="", I am not even sure where and how it get's the counts displayed in the captions:

29/1097

update:
additional entries came from admin menu, seems like a "huge" bug here:
colorbox_node.js (l101):

...
        var rel = $(this).attr('rel');
        if ($('a[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")').length > 1) {
            $related = $('a[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")');
            console.log($related);

console log

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
Issue summary: View changes
FileSize
Screenshot 2013-11-14 19.00.35.png24.12 KB
Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
Issue summary: View changes
Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
Issue summary: View changes
Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
FileSize
colorbox_module-rel_nofollow-2120319-9.patch1.71 KB

bad patch. see next comment

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
FileSize
colorbox_module-rel_nofollow-2120319-10.patch855 bytes

last patch contained stuff from something else. check this

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
FileSize
colorbox_module-rel_nofollow-2120319-11.patch862 bytes

sorry for the spam, just ran into another issue which is fixed by this updated patch as well:
currently every colorbox-node links are turned into galleries, even if rel is not set.

iLLin’s picture

iLLin CreditAttribution: iLLin commented

Have u checked with DEV? I believe this issue is fixed in DEV. Let me know if it's not.

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented

issue is not fixed completly in dev. My patch added two things: check for if($(this).attr('rel')) and changed the jquery selector from $('a[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")') to $('a.colorbox-node[rel="' + rel + '"]:not("#colorbox a[rel="' + rel + '"]")')

as well as the followed $related. there are multiple issues right now to be fixed. do you want to apply them manually or shall I create a new issue + patch for latest dev? I also believe that we should not add a whitelist for rel's, but add another css class to indicate wether that link belongs to a grouping or not. What do you think?

iLLin’s picture

iLLin CreditAttribution: iLLin commented

Hmm that's a good idea. How about a complete patch against DEV. Sounds like it's all related.

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
FileSize
colorbox_node-better-gallery-implementation-2120319-15.patch3.84 KB

[PATCH] better gallery implementation:
- fixed selectors to only match colorbox-node-gallery links for related items.
- added a class to handle colorbox gallery definition: colorbox-node-gallery;
in order to create a gallery you now have to define a link like this:
<a href="#" class="colorbox-node colorbox-node-gallery" rel="gallery1">Image</a>
- excluded duplicate anchors from showing up as a single item in the gallery:

   <ul>
       <li><a href="node/1" class="colorbox-node colorbox-node-gallery" rel="gallery1">Image</a></li>
       <li><a href="node/1" class="colorbox-node colorbox-node-gallery" rel="gallery1">Image</a></li>
       <li><a href="node/2" class="colorbox-node colorbox-node-gallery" rel="gallery1">Image</a></li>
  </ul>

-> this will create a gallery of
two items only, instead of three with 2 multiple targets.

this is very handy if you create listings in view that might have multiple fields + links to the same node. With his implementation the gallery will still work as expected instead of forcing you to alter the complete markup to only include one link.

patch created with git format-patch.

aze2010’s picture

aze2010 CreditAttribution: aze2010 commented

Thank you!

#15 Patch is working so far!!

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented

patch is waiting for commit for almost a month, any blockers here?

iLLin’s picture

iLLin CreditAttribution: iLLin commented

Waiting for more testers...

jncruces’s picture

jncruces
Primary language Spanish
Location Sevilla
CreditAttribution: jncruces commented

The patch #15 not worked for me.

Colorbox 7.x-2.4
Colorbox Node 7.x-3.2
Colorbox library 1.4.18

iLLin’s picture

iLLin CreditAttribution: iLLin commented

I'm just getting back to this and have started testing the patch locally. I have noticed that if you have a rel and you don't have the colorbox-node-gallery class, it still tries to add the pager. Then if you click next, nothing works. @Andre-B, do you experience this as well?

iLLin’s picture

iLLin CreditAttribution: iLLin commented

Im going to update DEV with this patch and a few other lingering things. @Andre-B I added one other check to this to remove the rel attribute but I don't like it as it changed the HTML markup. I think we should clone the link thats sent to the colorbox function without passing in the original to prevent that, matter of fact let me just do that, give me a sec and I will get this updated to DEV and then you can review.

iLLin’s picture

iLLin CreditAttribution: iLLin commented
Status: Needs review » Fixed

Committed: http://drupalcode.org/project/colorbox_node.git/commit/4b3f725

Thanks Andre-B for your work!

Status: Fixed » Closed (fixed)

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