Hi,

There should be an option to allow for absolute URLs. From an SEO perspective, having the Absolute URL will allow crawlers to know they can navigate to any link in the breadcrumb using https://. Otherwise there is a possibility that using relative URLs will set links to "/page1/page2" where the crawler can navigate to page1 by doing "http://domain.com/page1".

Having this feature will expand the functionality and allow site builders to choose what they want.

Here is a mymodule_system_breadcrumb_alter that was done for now. It would be preferred to have this in the configuration page found on admin/config/user-interface/easy-breadcrumb

It uses hook_system_breadcrumb_alter

CommentFileSizeAuthor
#14 easy_breadcrumbs_absolute_paths_interdiff_12_14.txt880 bytesnkoporec
#14 easy_breadcrumbs_absolute_path_2933559_14.patch4.02 KBnkoporec
#12 2933559-change-relative-absolute-urls-12.patch2.65 KBdrup16
#11 easy_breadcrumbs_absolute_path_interdiff_7_11.txt814 bytesnkoporec
#11 easy_breadcrumbs_absolute_paths_2933559_11.patch2.91 KBnkoporec
#7 2933559-change-relative-absolute-urls.patch1.95 KBdrup16

Comments

drupi17 created an issue. See original summary.

drup16’s picture

drup16 commented
Issue summary: View changes
drup16’s picture

drup16 commented
Priority: Major » Normal
Issue summary: View changes
drup16’s picture

drup16 commented
Issue summary: View changes
greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Have you had a chance to work on this at all? :)

~G

drup16’s picture

drup16 commented

@Greg,

No I have not had time to work on this. If i can get a patch written then I will try to upload it soon.

drup16’s picture

drup16 commented
Status: Active » Needs review
StatusFileSize
new 2933559-change-relative-absolute-urls.patch1.95 KB

Attaching the patch to allow for Absolute paths based on config. Currently the Home URL is displayed as "/", a separate patch can be added if that is a requirement.

drup16’s picture

drup16 commented

@Greg, The patch is based on the latest 8.x-1.x branch from the git repository.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

You rock! Looks perfect.

drup16’s picture

drup16 commented

Thanks, let's wait for people to test it before declaring it RTBC.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
StatusFileSize
new easy_breadcrumbs_absolute_paths_2933559_11.patch2.91 KB
new easy_breadcrumbs_absolute_path_interdiff_7_11.txt814 bytes

Tested your patch, but I found an issue.I found no configuration where to enable/disable absolute paths, so I create a small patch fix which adds a checkbox to module config. pages.

drup16’s picture

drup16 commented
StatusFileSize
new 2933559-change-relative-absolute-urls-12.patch2.65 KB

@nkoporec

Good catch, it helps when the correct patch is included. Slightly modified the title and description

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Can we add a config addition to the Schema file as well?

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
StatusFileSize
new easy_breadcrumbs_absolute_path_2933559_14.patch4.02 KB
new easy_breadcrumbs_absolute_paths_interdiff_12_14.txt880 bytes

Hi, thanks for the review.I added config schema and install schema as well.

drup16’s picture

drup16 commented

@nkoperec,

What branch did you use to generate the patch? I am pulled the 8.x-1.x branch and did not find language_path_prefix_as_segment in my easy_breadcrumb.schema.yml file

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented

I used 8.x-1.x branch , the language_path_prefix_as_segment is the last commit in the branch ( issue 2941922).

drup16’s picture

drup16 commented

Interesting because I downloaded easy_breadcrumb v8.x-1.6 via composer and it didn't have it. Looking at the 8.x-1.x branch, that has commits after tags/8.x-1.6, it appears the tag doesn't have the changes. I assume a new tag is coming soon and will include these changes on the next version.

Either way I have tested the patch and it works for me.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Yep Yep! for contrib, please use the git checkout of the module from the dev branch. The tagged releases are always quite a bit behind.

useernamee’s picture

useernamee commented

I have tested #14 patch. It applies nicely and works as it should.

useernamee’s picture

useernamee commented
Status: Needs review » Reviewed & tested by the community

  • Greg Boggs committed d037ad8 on 8.x-1.x authored by nkoporec 
    Issue #2933559 by nkoporec, drup16: Change relative to absolute URLs
greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Thanks everybody. This will be in the next release. I missed your issue credit there Lio. Let me know if you're worried about it and I'll figure something out to get you proper credit.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

drup16’s picture

drup16 commented

@Greg,

Just realized I never got credit for this. Is it fair for me to update this to issue the credit or do you have to do it? My patch from #12 handled the logic and #14 was added before I got a chance to add it myself.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented
greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Fixed. :)