ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
25.35k stars 2.17k forks source link

[Bug]: Pasting text with more than one link sets all links' href to the first link's href #4541

Open angelikatyborska opened 9 months ago

angelikatyborska commented 9 months ago

Which packages did you experience the bug in?

link

What Tiptap version are you using?

2.1.12

What’s the bug you are facing?

I am trying to copy-paste a piece of content that contains more than one link, and the links' text content can be interpreted as valid URLs. Below an example of such content:

--- start copying below ---

Some search engines:

--- stop copying above ---

When I copy-paste this to a TipTap editor, every link but the first one has a wrong href attribute - all links have a href attribute equal to the href attribute of the first link. See a screen recording below.

Basically on paste, TipTap changes this:

Some search engines:
- DuckDuckGo: <a href="https://duckduckgo.com/">https://duckduckgo.com/</a>
- Bing: <a href="https://www.bing.com/">https://www.bing.com/</a>

To this:

Some search engines:
- DuckDuckGo: <a href="https://duckduckgo.com/">https://duckduckgo.com/</a>
- Bing: <a href="https://duckduckgo.com/">https://www.bing.com/</a>

This only happens for links whose text content is also an URL - it doesn't happen when links are for example like [DGG](https://duckduckgo.com/)

What browser are you using?

Firefox

Code example

No response

What did you expect to happen?

I expect all links to always have the same href attribute as on the source page from which I copied them.

Anything to add? (optional)

https://github.com/ueberdosis/tiptap/assets/7852553/8a3f6431-a39a-4cc1-95e0-c67e6057fcd8

I didn't prepare a CodeSandbox because this behavior can be observed right now on TipTap's official page for the link extension, as seen on the video.

Did you update your dependencies?

Are you sponsoring us?

C-Hess commented 8 months ago

Cannot reproduce as of 2.2.0-rc.4

Likely fixed by #4523

angelikatyborska commented 8 months ago

Likely fixed by https://github.com/ueberdosis/tiptap/pull/4523

I don't think so because that PR was part of the 2.1.12 release where I'm definitely still experiencing this bug.

I'll try out 2.2.0-rc.4 and report back.

angelikatyborska commented 8 months ago

@C-Hess The bug exists both in 2.2.0-rc.4 and in 2.1.12. Here's a reproduction codesandbox for 2.2.0-rc.4: https://codesandbox.io/s/dazzling-rain-mpqrh8?file=/src/index.js

https://github.com/ueberdosis/tiptap/assets/7852553/b1580374-61b4-4291-a429-942e4d0b3734

C-Hess commented 8 months ago

Interesting, I'll take a look again, sorry.

C-Hess commented 8 months ago

I see now, I mistakenly forgot to clean my build when testing.

Looks like this paste handler is to blame: https://github.com/ueberdosis/tiptap/blob/42039c05f0894a2730a7b8f1b943ddb22d52a824/packages/extension-link/src/link.ts#L151C1-L186C6

addPasteRules() {
    return [
      markPasteRule({
        find: text => find(text)
          .filter(link => {
            if (this.options.validate) {
              return this.options.validate(link.value)
            }

            return true
          })
          .filter(link => link.isLink)
          .map(link => ({
            text: link.value,
            index: link.start,
            data: link,
          })),
        type: this.type,
        getAttributes: (match, pasteEvent) => {
          const html = pasteEvent.clipboardData?.getData('text/html')
          const hrefRegex = /href="([^"]*)"/

          const existingLink = html?.match(hrefRegex)

          if (existingLink) {
            return {
              href: existingLink[1],
            }
          }

          return {
            href: match.data?.href,
          }
        },
      }),
    ]

It only grabs the first href of your entire clipboard payload I believe. This will have to modified to support multiple links within the same clipboard payload.

It also looks like #4523 was actually not merged into the develop branch, and even if it was it looks like it has the same problem...

C-Hess commented 8 months ago

Causing change #3975 @bdbch .

I think one solution could be to add some sort of option in markPasteRule so that it will NOT override an existing mark if the mark type is identical (aka, if it was already parsed via parseHTML, the paste rule should not apply)

C-Hess commented 8 months ago

Created a draft PR. need to perform further testing of side effects and write tests before publishing

101budspencer commented 8 months ago

Fix there already?

Arnaud-SR commented 7 months ago

Created a draft PR. need to perform further testing of side effects and write tests before publishing

Any news about that ?

C-Hess commented 7 months ago

Sorry, I'll try and wrap that PR up the next few days

101budspencer commented 7 months ago

Okay let us know.

101budspencer commented 7 months ago

Sorry, I'll try and wrap that PR up the next few days

Can you do it?

C-Hess commented 7 months ago

Published. Ready for review

101budspencer commented 6 months ago

How is it? @bdbch @C-Hess @Arnaud-SR

Arnaud-SR commented 5 months ago

Published. Ready for review

Any news about that ?

giuseppeagostini-hotmart commented 3 months ago

any update? In version 2.2.4 it is still giving me problems @C-Hess

v-tsys commented 3 months ago

Yo! Is there any ETA for this fix?