kontent-ai / delivery-sdk-js

Kontent Delivery SDK for Javascript
https://kontent.ai
MIT License
50 stars 34 forks source link

Rich text resolver > URL resolver - Incorrect link text passed when formatting is used #351

Closed MilanLund closed 2 years ago

MilanLund commented 2 years ago

Brief bug description

When a link inside a rich text element uses formatting, only a part of the link text is passed in the urlResolverAsyc method.

Repro steps

  1. In the Kontent app, in a rich text element, create a link and add formatting inside the link text (ie. italic or bold). image
  2. When resolving URLs in a Rich text resolver, only the text in front of the formatting is passed in the linkText variable. image

Expected behavior

The whole link text is passed in the method.

Test environment

Enngage commented 2 years ago

Hey @MilanLund,

Thank you for reporting the issue! I've made some changes to the parser and it should now include the entire link content as a plain text. Can you try the "@kentico/kontent-delivery-node-parser": "2.0.0" ?

MilanLund commented 2 years ago

Hi @Enngage, I am afraid your changes broke it a little bit more :) I tried version 2.0.0 in my project and got TypeError: Cannot read property 'value' of undefined on almost every page. I dug a bit in your code and found an issue. Here you are testing if there are any childNodes: https://github.com/Kentico/kontent-delivery-node-parser/blob/48456ca8380ca329a3b5509bb68ed1d9c12301f7/lib/parser/implementation/node-async-parser.ts#L233 And then if there are none you are obtaining a first childNode which obviously is undefined: https://github.com/Kentico/kontent-delivery-node-parser/blob/48456ca8380ca329a3b5509bb68ed1d9c12301f7/lib/parser/implementation/node-async-parser.ts#L238

Enngage commented 2 years ago

Thanks @MilanLund ! Yeah, you're right ;) I've updated the condition in 2.0.1, can you try again?

I was changing the code quite a bit (due to parse5 API changes) and didn't notice this as test were passing correctly. I'm honestly not sure what are all possible link syntaxes and it's strange that I didn't see the same error as you did. I only wonder how your links differ from the ones I have in tests:)