-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix TinyMCE edit link when link contains html #1814
Fix TinyMCE edit link when link contains html #1814
Conversation
0a71c70
to
b995ba7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments in silverstripe/silverstripe-cms#2993 (review) apply to this PR as well.
b995ba7
to
bd2518d
Compare
I have updated the fix by moving the selection to the link element. This does not need the other PR. I have tested this with internal and external linking, it worked for me as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this works great locally. Just one tiny change to make.
client/src/legacy/TinyMCE_sslink.js
Outdated
@@ -35,6 +35,9 @@ const plugin = { | |||
|
|||
// Callback for opening the edit link dialog form | |||
function openLinkDialog() { | |||
// Find "a" node, issue https://github.com/silverstripe/silverstripe-cms/issues/2439 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Find "a" node, issue https://github.com/silverstripe/silverstripe-cms/issues/2439 | |
// Find "a" node (we might have clicked on a child element e.g. "span") |
There's no need to list the issue - git blame will inevitably lead us back to this PR and then to that issue if we really need that level of context on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done updated
bd2518d
to
03e0286
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, works well locally. Thanks for this!
Description
If you have a link in tinymce like
<a href=""><span>Title</span></a>
, then you can edit the link. The HTML inside the link tag can be any tag.The issue is that the selected node is the inside HTML tag instead of the link tag.
I'm not 100% keen on the solution using
while
but this is legacy code, so might be ok! Alternative code is welcomed :)The PR silverstripe/silverstripe-cms#2993 depends on this PR.
Manual testing steps
Issues
Pull request checklist