blipson89 / Synthesis

Synthesis is a universal object mapper for Sitecore
MIT License
75 stars 25 forks source link

Bug: HyperlinkField.Href not working for Media using Insert Link #109

Open jon-morgan opened 1 year ago

jon-morgan commented 1 year ago

bug

Using HyperlinkField.Href will give the wrong URL when the link field is a media item that has been added using 'Insert Link' (rather than 'Insert Media Link').

If I add using 'Insert Media Link', the raw value of the field is "<link linktype=\"media\" id=\"{E06C8962-B4D8-45E3-86D2-46B5D6FB60ED}\" />" - the linktype is "media" and the Href is "/-/media/song.mp3", which is fine.

If I add using 'Insert Link', the raw value of the field is "<link linktype=\"internal\" id=\"{E06C8962-B4D8-45E3-86D2-46B5D6FB60ED}\" />" - the linktype is "internal".

The Href is "/sitecore/media-library/song", so this is not valid (505). However, the RenderedValue is correct and is "<a href=\"/-/media/song.mp3\">song</a>"

The Href should also be "/-/media/song.mp3"

I'm using Sitecore 9.3 and Synthesis 9.1.6

blipson89 commented 1 year ago

Hi @jon-morgan,

Thanks for reporting this! I'll do a bit of digging this weekend and get back to you with what I find.

Thanks, Ben

jon-morgan commented 1 year ago

Hi Ben,

Great, thanks.

If it's helpful, what I've done myself is to check the media path.

So in FieldUtility.GetGeneralLinkHref(LinkField field), the switch(field.LinkType) is insufficient test because a media case may also be: if (field.LinkType == "internal" && field.TargetItem != null && field.TargetItem.Paths.IsMediaItem)

And thanks for your fantastic work on Synthesis as a whole, such a huge help.

Ta,

On Fri, 11 Nov 2022 at 00:25, Ben Lipson @.***> wrote:

Hi @jon-morgan https://github.com/jon-morgan,

Thanks for reporting this! I'll do a bit of digging this weekend and get back to you with what I find.

Thanks, Ben

— Reply to this email directly, view it on GitHub https://github.com/blipson89/Synthesis/issues/109#issuecomment-1311073303, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMFABO3WHV4KMGMAZWDTIE3WHWG65ANCNFSM6AAAAAAR4KPXRY . You are receiving this because you were mentioned.Message ID: @.***>

-- http://www.tearfund.org/ Jon Morgan IT Systems Developer [FIT- IT Supporter Systems] tel: 020 3906 3261 <02039063261> mob/txt: 07858 999 225 <07858999225> skype: jon.morgan 100 Church Road, Teddington, TW11 8QE http://goo.gl/8Zsslu

-- Tearfund, registered charity in England and Wales (265464) and Scotland (SC037624)  A company limited by guarantee. Registered in England and Wales company number 00994339. Registered office: 100 Church Road, Teddington, TW11 8QE.

We have updated our privacy policy to make it easier to stay in touch by post, simplified the number of sign ups we ask you for and provided more information on how we use social media marketing.

If you would like to find out more about our commitment to your personal data then please read our privacy policy https://www.tearfund.org/en/admin/terms_and_conditions/privacy_policy/

blipson89 commented 1 year ago

Hi @jon-morgan,

I have a fix, but I'm a bit hesitant to release it as there may be some solutions out there that could actually be expecting this behaviour, not considering it to be a bug (I've seen people do silly things with the media library).

I pushed the fix up as Synthesis 9.1.7-beta2 which can be downloaded in NuGet if you select "Include prerelease" image

I'm going to do a bit of digging in the meantime to try to figure out if there are any possible use cases what may legitimately want the existing behaviour, but 9.1.7-beta2 should hold you over in the meantime.

Is there a reason you're using internal links and not media links for this?

Thanks, Ben

jon-morgan commented 1 year ago

Hi Ben,

Sure, I understand. Thanks for the fix, that's great.

There's no reason for using internal links except the editors were using them without realising why it didn't work. I was initially looking to prevent that option so they couldn't do that when I saw RenderedValue resolved it correctly. So I thought it must be viable instead.

Ta

On Sat, 12 Nov 2022 at 21:35, Ben Lipson @.***> wrote:

Hi @jon-morgan https://github.com/jon-morgan,

I have a fix, but I'm a bit hesitant to release it as there may be some solutions out there that could actually be expecting this behaviour, not considering it to be a bug (I've seen people do silly things with the media library).

I pushed the fix up as Synthesis 9.1.7-beta2 which can be downloaded in NuGet if you select "Include prerelease" [image: image] https://user-images.githubusercontent.com/4602966/201495398-359e412e-17cf-49e8-81f4-916c65229ed5.png

I'm going to do a bit of digging in the meantime to try to figure out if there are any possible use cases what may legitimately want the existing behaviour, but 9.1.7-beta2 should hold you over in the meantime.

Is there a reason you're using internal links and not media links for this?

Thanks, Ben

— Reply to this email directly, view it on GitHub https://github.com/blipson89/Synthesis/issues/109#issuecomment-1312577625, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMFABOZZOXGDFYQAEDSDSH3WIAETRANCNFSM6AAAAAAR4KPXRY . You are receiving this because you were mentioned.Message ID: @.***>

-- http://www.tearfund.org/ Jon Morgan IT Systems Developer [FIT- IT Supporter Systems] tel: 020 3906 3261 <02039063261> mob/txt: 07858 999 225 <07858999225> skype: jon.morgan 100 Church Road, Teddington, TW11 8QE http://goo.gl/8Zsslu

-- Tearfund, registered charity in England and Wales (265464) and Scotland (SC037624)  A company limited by guarantee. Registered in England and Wales company number 00994339. Registered office: 100 Church Road, Teddington, TW11 8QE.

We have updated our privacy policy to make it easier to stay in touch by post, simplified the number of sign ups we ask you for and provided more information on how we use social media marketing.

If you would like to find out more about our commitment to your personal data then please read our privacy policy https://www.tearfund.org/en/admin/terms_and_conditions/privacy_policy/