Problem/Motivation

When clicking an anchor link within CKEditor5, the browser attempts to navigate to the anchor link. If the link points to an anchor that exists on the edit page (e.g. #main-content), the browser scrolls up/down to that anchor’s position on the page.

This is not the expected behavior. When I click on a non-anchor link (be it an internal or external link), a tooltip pops up which displays the URL (which is linked), an Edit button, and an Unlink button. I can then choose to navigate to the link’s URL by clicking on the URL. This is the behavior I expect to see when clicking on an anchor link within CKEditor5. While this tooltip also appears when the link is an anchor link, the browser navigates to the anchor in addition to opening the tooltip.

See attached screen cast for a demonstration of the behavior I’m describing on a fresh Drupal 10.1 site, but I have seen this happen on a 9.5 site as well. I attempted to reproduce this on this CKEditor5 demo page and was unsuccessful, which is why I’m reporting it here.

Steps to reproduce

  1. Edit a page that requires you to scroll down a bit before you get to a WYSIWYG editor.
  2. In a CKEditor5 editor, select some text and link it to #main-content.
  3. Click on the linked text.
  4. Observe that the browser scrolls up to the #main-content anchor near the top of the node edit form.

Proposed resolution

Update the scope of the selector passed to the fragment link event listener in core/misc/form.es6.js to exclude links which are inside a WYSIWYG editor.

Issue fork drupal-3339741

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

agarzola created an issue. See original summary.

agarzola’s picture

Issue summary: View changes
agarzola’s picture

Issue summary: View changes
wim leers’s picture

Title: Clicking on an anchor within CKEditor navigates to anchor » [upstream] Clicking on an anchor link within CKEditor 5 navigates to anchor
Issue tags: +Needs upstream bugfix

Wow, superb find! Looks like there's no issue for this yet upstream: https://github.com/ckeditor/ckeditor5/issues?q=is%3Aissue+link+anchor+is.... Would you mind creating it? 🙏

agarzola’s picture

I have not been able to reproduce the issue in any of their demos, actually. You can try it for yourself: https://ckeditor.com/ckeditor-5/demo/feature-rich/

That is the reason I have not reported it upstream and instead reported it here. Happy to open a ticket, though, if we’re able to reproduce it in a non-Drupal setting.

agarzola’s picture

Title: [upstream] Clicking on an anchor link within CKEditor 5 navigates to anchor » Clicking on an anchor link within CKEditor 5 navigates to anchor

Removing the [upstream] badge until we can confirm that this is reproducible upstream.

agarzola’s picture

Version: 10.1.x-dev » 9.5.x-dev
Component: ckeditor5.module » javascript
Assigned: Unassigned » agarzola
Issue tags: -ckeditor5, -Needs upstream bugfix +form.js

I found the culprit. It actually does not have to do with the CKEditor module at all, but rather form.js. This file (found at core/misc/form.js) adds a click event listener to all anchor links on the page which will trigger a fragment interaction event, causing the browser to navigate to the anchor target. It targets all anchor links indiscriminately, without regard to where it might be located:

  /**
   * Binds a listener to handle clicks on fragment links and absolute URL links
   * containing a fragment, this is needed next to the hash change listener
   * because clicking such links doesn't trigger a hash change when the fragment
   * is already in the URL.
   */
  $(document).on(
    'click.form-fragment',
    'a[href*="#"]',
    debouncedHandleFragmentLinkClickOrHashChange,
  );

I believe the solution here is to amend the selector passed in the second argument to $().on() so that links found inside a WYSIWYG editor are excluded from the query. I will be submitting merge requests for Drupal 9.5.x-dev and 10.1.x-dev shortly.

Since this doesn’t actually have anything to do with the CKEditor core module, I changed the Component field on this issue to “javascript”, though perhaps there is a more appropriate selection?

agarzola’s picture

Title: Clicking on an anchor link within CKEditor 5 navigates to anchor » Clicking on an anchor link within a WYSIWYG navigates to anchor
Issue summary: View changes
agarzola’s picture

Status: Active » Needs review

I could use some guidance on the proper way to submit MRs against multiple branches. Should I create a child issue for the 10.1.x-dev branch and create an MR from there?

agarzola’s picture

Version: 9.5.x-dev » 10.1.x-dev
Status: Needs review » Active

Kind friends showed me the way. I’m closing my MR, changing the target version on this issue, and submitting a new MR that targets 10.1.x.

agarzola’s picture

Status: Active » Needs review
sonam.chaturvedi’s picture

StatusFileSize
new1.86 MB
new2.06 MB

Verified and tested MR!3486 on 10.1.x-dev. Patch applied cleanly and works fine.

Test Results
1.Now user is not navigated to an anchor when a user clicks on a fragment link within a WYSIWYG editor.
2.Only tooltip is shown with the URL and now user can choose whether navigate to the link’s URL by clicking on the URL in tooltip.

Please find attached before and after MR video.

agarzola’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the review @sonam.chaturvedi.

wim leers’s picture

Status: Reviewed & tested by the community » Needs work

Oh wow — so it's a regression on the Drupal core side simply because CKEditor 4 used <iframe>s, and CKEditor 5 doesn't! 🤯

What a magnificently obscure bug, and epic work shepherding this in the right direction, @agarzola! 👏

Unfortunately, I think the solution is a bit too broad, even though it is probably harmless on 99% of sites. I left a suggestion that will allow us to make it harmless on 100% of sites.

agarzola’s picture

Oh wow — so it's a regression on the Drupal core side simply because CKEditor 4 used

s, and CKEditor 5 doesn't! 🤯

What a magnificently obscure bug, and epic work shepherding this in the right direction, @agarzola! 👏

That’s really helpful context about why it wasn’t a problem before, and those are very kind words, Wim Leers! ☺️
agarzola’s picture

Status: Needs work » Needs review
StatusFileSize
new2.31 MB

I did some digging in an attempt to find alternative ways to limit the selector’s scope, but I found that what is proposed in the MR is not only adequate, but the correct solution.

My original concern (and I believe it’s @wimleers’s concern as well) was that perhaps other text which may potentially include one or more legitimate anchor links might get included inside of .form-textarea-wrapper (as a sibling of the textarea element and WYSIWYG editor). I can only think of one such element that might be included and that is helper text, so I did a test locally and it turns out that helper text is not injected into the .form-textarea-wrapper container element, so it is not at risk of getting left out of form.js’s purview with the query selector in my merge request. I have attached a new video showing this in action. The video shows my fix preventing the anchor link within the CKEditor UI from working (as intended). The field has helper text that includes an anchor link. Clicking on the anchor link correctly navigates to the anchor, as intended.

Respectfully, I am marking this back as Needs review and marking the conversation as resolved, as I believe that the solution in the MR is adequate. Please let me know if this is not proper procedure to get more eyes on this.

Thanks!

nayana_mvr’s picture

StatusFileSize
new1.04 MB
new2.86 MB

Verified the MR!3486 and tested it on Drupal version 10.1.x. After applying the patch, 2 scenarios were tested :-
1. When the anchor is inside CKEditor5 - On clicking the anchor link, a tooltip pops up which displays the URL (which is linked), an Edit button, and an Unlink button. On Clicking the link, the edit page will open in a new browser navigating to the anchor’s position on the page.
2. When the anchor is outside CKEditor5( eg., help text as mentioned in #21) - On clicking the anchor link, it will navigate/ scroll to the anchor’s position on the page. Which means the patch doesn’t affect anchor links outside CKEditor5.

So I think the patch works fine and I have added the before and after screen recordings for reference. Need RTBC+1

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

So seems like it may not be an upstream issue.

MR does fix the issue but will need a test case.

richarddavies’s picture

I just upgraded to CKEditor 5 and I've been troubleshooting a very similar situation where clicking on some links in the editor caused the browser to instantly navigate to the link. Although my root cause was different than what is described in this original issue, I'm leaving a comment here in case it helps anyone else.

What was different in my case was that my links didn't contain an anchor and it only happened when editing certain pages on my site. It was really difficult to pinpoint exactly what conditions were causing this behavior, but I finally determined that the issue was caused by our Google Tag Manager code which tracks clicks on outbound links. We had tried to exclude GTM on our admin/editor pages, but had overlooked some so GTM was unintentionally loading when editing certain types of pages. After reconfiguring GTM to exclude those pages I can now click on the links without navigating to them.

agarzola’s picture

Status: Needs work » Active
markdorison’s picture

Status: Active » Needs review
agarzola’s picture

Status: Needs review » Active

This is still active, as I'm working getting my test to pass.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

hitesh.koli’s picture

#24 works for me.

hitesh.koli’s picture

StatusFileSize
new55.51 KB

GTM setting for tracking only anonymous users.

Google tag > Conditions > User Role > Select `anon`

acbramley’s picture

Triaged as part of BSI.

Looks like Nightwatch tests were added, but the MR needs a reroll. I'll attempt to change the MR target and rebase.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.