Image loses link after drag and drop or copy/paste

[iseulde]

To reproduce, insert an image with a link, drag it to another paragraph. The link is gone.

[afercia]

Replying to avryl:
some additional details:

The link is gone.

it depends, if your linked image is inside a paragraph which contains text before and after the linked image, after you drag the image in a new position, the link will still be there in its original position and it will be "empty". TinyMCE won't clean up this empty link because it actually contains a line break, so your content will be saved and it will contain empty links, which is bad :)

To be more clear, this:

<a href="http://www.somelink.com"></a>

gets removed by TinyMCE when it runs its "cleanup" but this:

<a href="http://www.somelink.com">
</a>

doesn't.

About dragging, when you click a linked image, TinyMce shows you the current selection in the statusbar "path" as:
p » a » img
so you're actually selecting the image element and when you drag, you drag the img element.
The only way I'm aware of to drag the linked image together with its link, is to actually select the a element so just click the "a" in the TinyMCE statusbar path, and it will change to:
p » a
drag now to another paragraph and linked image will be moved correctly.

[SergeyBiryukov]

Related: #28357

[magdigit]

Why do we have to implement an additive like TinyMCE?
The WP-editor was sufficient enough, before 3.9!
Never had problems with drag&drops images with links...

After further reading in google, I just found out that TinyMCE is totally broken with the introduction of WP 3.9!!
And WP3.9 deleted the good features, like providing the resizing-scale-window, and the simple drag-drop system for adiacent images with each their own links.

I truly hope the developers will sincerely consider bringing this features back!!

[magdigit]

Replying to SergeyBiryukov:

Related: #28357

Why do we have to implement an additive editor like TinyMCE?
The WP-editor was sufficient enough, before 3.9!
I never had problems with drag&drops images with links...

After further reading in google, I just found out that TinyMCE is totally broken with the introduction of WP 3.9!!
And WP3.9 deleted the good features in its editor, like providing the resizing-scale-window, and the simple drag-drop system for adiacent images with each their own links. So I feel kind of 'driven in the corner'...

I truly hope the developers will sincerely consider bringing this features back!!

[iseulde]

WordPress has always used TinyMCE...

[magdigit]

Replying to avryl:

WordPress has always used TinyMCE...

Sorry dear Avril, I am not a real developer, but just an illustrator who created a platform to publish her work.

Before these problems I never heard of a thing called TinyMCE. So I studied for an hour on this new thing.

But the fact remains, whether WP uses his own TinyMCE or not, that since WP 3.9 two dear features suddenly disappeared... And I hope to convince the wonderful develop-people to bring these back.
Best wishes from Amsterdam

[ericlewis]

Replying to magdigit:

But the fact remains, whether WP uses his own TinyMCE or not, that since WP 3.9 two dear features suddenly disappeared... And I hope to convince the wonderful develop-people to bring these back.
Best wishes from Amsterdam

We hear you loud and clear, and will be looking into it.

Greetings from Brooklyn

[iseulde]

#28357 was marked as a duplicate.

[azaozz]

Replying to afercia:

About dragging, when you click a linked image, TinyMce shows you the current selection in the statusbar "path" as:
p » a » img
so you're actually selecting the image element and when you drag, you drag the img element.
The only way I'm aware of to drag the linked image together with its link, is to actually select the a element so just click the "a" in the TinyMCE statusbar path, and it will change to:
p » a
drag now to another paragraph and linked image will be moved correctly.

Yep, this is the proper way to drag an image wrapped in a link. However it is too complex for most users. We will need to make it work automatically. Perhaps something like:

• On dragstart check if image is selected, check if parentNode is a link with no other children, store ref to it.
• On dragend check if the image was actually moved in the dom, move the old parentNode (link) to the new location and append the image in it.

In theory the same approach should work for dragging images with captions. Big plus is that both dragstart and dragend are not fired when dragging a file from the OS into the browser window.

[azaozz]

#28357 was marked as a duplicate.

[afercia]

Replying to azaozz:

Yep, this is the proper way to drag an image wrapped in a link. However it is too complex for most users.

thx azaozz :)
I agree, wondering if most users even know what a "dl", "dt"or even a "p" is. Tinymce "path" is maybe one of its most overlooked features, though I find it very useful sometimes. Worth nothing that it gets a bit confused by media views "div" tag, showing a div in the path that can be a bit misleading for users. See attached screenshot.

[azaozz]

Replying to afercia:

I agree, wondering if most users even know what a "dl", "dt"or even a "p" is. Tinymce "path" is maybe one of its most overlooked features, though I find it very useful sometimes.

Yeah, the "path" is really helpful in some cases. It is the best/only way to select parent nodes. Not sure we can improve that for images with captions though. Can try limiting it to only the <dl> or the wrapper div.

Worth nothing that it gets a bit confused by media views "div" tag, showing a div in the path that can be a bit misleading for users. See attached screenshot.

Opened #28567 for that.

[SergeyBiryukov]

Related: #28826

[iseulde]

Related: #30985.

[iseulde]

@wonderboymusic Not sure why "looses" is incorrect. :)

[archon810]

Any updates? This is tripping us up almost every day, with images ending up without links to larger versions and just unclickable thumbnails. A pretty bad user-facing bug if I may say so myself.

[afercia]

Replying to archon810:

Any updates? This is tripping us up almost every day

That's why the "path" in the TinyMCE status bar is so important :) I agree TinyMCE needs some improvements when handling selections but also editors should be informed that they always have the ability to check what's currently selected just having a look at the TinyMCE status bar and clicking there on the element they want to be selected.

[archon810]

Replying to afercia:

Replying to archon810:

Any updates? This is tripping us up almost every day

That's why the "path" in the TinyMCE status bar is so important :) I agree TinyMCE needs some improvements when handling selections but also editors should be informed that they always have the ability to check what's currently selected just having a look at the TinyMCE status bar and clicking there on the element they want to be selected.

I'm not sure what that has to do with the problem at hand. Our issue is that just moving an image strips the link attached to that image. It is a destructive behavior and not something obvious that my editors can watch out for unless they remember to manually check every image in the post. I'd really love to hear why it's not being given a higher priority to be fixed.

[valendesigns]

@archon810 This is an issue that needs to be handled upstream from within the TinyMCE project. This is likely why you feel it's not getting the attention you believe it deserves. Fortunately, I've found a solution, and tested that it's not already been fixed in the next version of TinyMCE. As well, I've submit a pull request upstream and now it's a waiting game until it makes its way into the WP Core. That is if my fix is correct, which I cannot guarantee since this is the first time I've interacted with that library and am not very familiar with it.

​https://github.com/tinymce/tinymce/pull/468

[archon810]

Replying to valendesigns:

@archon810 This is an issue that needs to be handled upstream from within the TinyMCE project. This is likely why you feel it's not getting the attention you believe it deserves. Fortunately, I've found a solution, and tested that it's not already been fixed in the next version of TinyMCE. As well, I've submit a pull request upstream and now it's a waiting game until it makes its way into the WP Core. That is if my fix is correct, which I cannot guarantee since this is the first time I've interacted with that library and am not very familiar with it.

​https://github.com/tinymce/tinymce/pull/468

Thanks for the effort. I hope it's the right fix, and it gets approved by TinyMCE. Then I can just apply it locally without waiting for Wordpress, at least until it's part of WP.

[magdigit]

But in WP 4.1 the attachment of links to images when moved, works now correct! How is that possible?

[afercia]

Replying to magdigit:

But in WP 4.1 the attachment of links to images when moved, works now correct! How is that possible?

It depends if you performed a correct selection or not. If in the TinyMCE status bar you see something like:

p » a » img

then you've actually selected the image, not the link: drag and you will drag just the image, the link will stay in its original place.
So, just click on the "a" in the path p » a » img and it will change in p » a
Now you've selected the link and the image. Dragging now will drag both. That's what I was trying to explain in my previous comment. Agreed that's a workaround but as @valendesigns explained, the real issue should be fixed upstream.

[iseulde]

Thank you for the pull request @valendesigns. I'm not sure if a potential fix for TinyMCE should be added to Quirks.js or not, but if it should, it should be targeted at all browsers. Your patch would only work for IE and WebKit as far as I can see. That file is mainly meant to fix browser specific issues with contenteditable.

We can try to fix this in WordPress though, and such a fix could also be used for captions. Attaching a patch now. The patch would also fix copy/paste, not just drag and drop.

[iseulde]

Let's see what azaozz thinks about this. The only problem with the link around an image is that, in rare cases, it could also be wrapped in something else like a strong tag first. So <a><strong><img>.... In another rare case the link might span over more than just the image.

Patch would also fix #28003 and #30985.

[iseulde]

In any case this would be good for captions, I don't see any problems with that right now.

[valendesigns]

@iseulde That makes sense, thank you for the clarification on the pull request and Quirks.js. I've closed the PR since you've pointed out there is a file which already exists where we can solve this issue in the Core. However, your patch doesn't fix moving captions. It will move the captions content, but not the actual caption itself. So this does not fix #28003, and if we're going to solve captions we should do it in that ticket. IMHO We need to solve images inside anchors first.

I added 28272.1.diff​ that changes a few things from your patch like bring back the code that prevented moving images in captions (now inside the function), added code for those rare instances where there is a wrapping node like strong or em, and also added checks to make sure the parent anchor doesn't have text content. As well, I've renamed the function to selectImageParent and moved it up with the rest of the image functions earlier in the file.

[iseulde]

#31400 was marked as a duplicate.

[valendesigns]

I've refreshed the patch because it did apply cleanly anymore.

[iseulde]

However, your patch doesn't fix moving captions.

Works in Chrome, but seems to break in FF. Not sure why, because selecting a link during dragstart does work. It does seem to select the right thing though. Maybe it doesn't update fast enough. What browser were you using?

[valendesigns]

@iseulde I was using Chrome & FF in OSX and both did not work when trying to move captions. How does the latest patch solve the image wrapped in a link issue for you? Are you able to move them consistently in multiple browsers?

[iseulde]

Yes, selecting the link works fine. But I'd like to know why and what's different from that div.
I also think we shouldn't try to select links above the parent. If we do that we'd have to check higher up too with a loop and check for inline elements, but I think it's too rare. Instead maybe we should make sure the link isn't wrapped in garbage.

[iseulde]

#30985 was marked as a duplicate.

[valendesigns]

@iseulde I believe the only difference between 28272.1.diff & 28272.2.diff was I removed some tab whitespace that I accidentally added. They are the same code wise and sorry for the really long delay in getting back to you on that. Ot were you asking what's different between yours and mine?

I guess we need to decided what's left for this ticket. What does and doesn't work at this point.

[helen]

Too late for 4.2, not a regression in trunk. Try again in 4.3.

[azaozz]

Selecting a different node on dragstart will always be problematic as it is a bit of a "Catch 22" :)

We need to change the selection before the dragging starts, but the event is fired when the dragging starts. Also, in contenteditable images are draggable but block tags are usually not.

Imho this will work best as described in comment 9:

• On dragstart check if image is selected, check if it is wrapped in a link or caption node, store ref to it.
• On dragend check if the image was actually moved in the DOM, move the previous wrapper to the new location and append the image in it.

[iseulde]

I guess we should just fix the case where the link is the image's parent without anything in between (almost all cases). If there are other nodes in between it breaks quite a few other things (no link field in the image modal).

[iseulde]

In 35261:

TinyMCE: Drag and drop link with image

Make sure images don't loose their link after drag and drop.

Fixes #28272.