There are some cases when it would be good if ng_lightbox could check if the internal path of an alias path matches the patterns rules, when checking if that link should link to an ng_lightbox.

For example, you may set the rule where all the paths that matches "node/*" should link to ng-lightboxes. But your paths are allready alised (e.g. /home instead of /node/1), in this case the path don't match the patterns.

CommentFileSizeAuthor
#8 2771255-8-ONLY_FOR_2771255.patch1.38 KBNWOM
#6 interdiff-2771255-2-6.txt473 bytesradubutco
#6 aliased_paths_not-2771255-6.patch1.38 KBradubutco
#2 aliased_paths_not-2771255-2.patch1.36 KBradubutco
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

radubutco created an issue. See original summary.

radubutco’s picture

radubutco CreditAttribution: radubutco commented
FileSize
aliased_paths_not-2771255-2.patch1.36 KB

The code checks if the path has an alias which matches the patterns.
I've added a code which check if the path which is allready an alias and the internal path matches the patterns.

radubutco’s picture

radubutco CreditAttribution: radubutco commented
Assigned: radubutco » Unassigned
Status: Needs work » Needs review
benjy’s picture

benjy CreditAttribution: benjy commented
Issue tags: -path alias +Needs tests

This looks good thanks!.

Lets also add a test and then we can get this committed.

mariancalinro’s picture

mariancalinro CreditAttribution: mariancalinro at Dream Production commented
Status: Needs review » Needs work

Setting it to NW per benjy's comment.

radubutco’s picture

radubutco CreditAttribution: radubutco commented
FileSize
aliased_paths_not-2771255-6.patch1.38 KB
interdiff-2771255-2-6.txt473 bytes

A new condition was needed to the previous patch, because the path for home page has always link to nb_lightbox.

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented
Status: Needs work » Needs review

Do you want to try add a new test here and we can get it into the next release?

NWOM’s picture

NWOM CreditAttribution: NWOM commented
FileSize
2771255-8-ONLY_FOR_2771255.patch1.38 KB

#6 works great. Thank you! Also for those on D9 that need a a compatible patch with #3150684: Add D9 compatibility for NG Lightbox, I have made a temporary fix. I'll hide the file, since it should not be committed.

NWOM’s picture

NWOM CreditAttribution: NWOM commented
Status: Needs review » Needs work

Never mind. #6 has a problem with multilingual sites. If you use the path /*/node/10 to include all languages as an example, it does not work for aliased nodes still.

Edit: It also doesn't work with non-aliased URLS without wildcards. For example: /en/node10.