Skip to content
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

Support text selection when editor is in a shadow root #7846

Closed
wants to merge 5 commits into from

Conversation

ywsang
Copy link
Contributor

@ywsang ywsang commented Aug 14, 2020

Suggested merge commit message (convention)

Other (engine): Use the source element root (document vs. shadow root) to determine which "root" to perform getSelection on.
Other (core): Pass the sourceElementRoot to the Editing Controller.


Additional information

Related to #3891 (and more generally, #1483).

In order to know whether the editor is in shadow DOM or not, and maintain a reference to the shadow root in the former case, this PR uses a sourceElementRoot option set in editor's config upon creation. If the editor is created in a shadow root, the shadow root should be passed in this option. If not provided, this option defaults to the document.

@Reinmar Reinmar self-requested a review August 17, 2020 07:56
@Comandeer
Copy link
Member

@ywsang, is such a solution compatible with browsers other than Blink-based ones? AFAIR the shadowRoot.getSelection() method is Chrome-only and there is no clear consensus how to handle selection in shadow roots.

@Reinmar
Copy link
Member

Reinmar commented Sep 15, 2020

That'd be good to know, indeed. Also, in order to actually test the PR a manual test would be helpful.

Copy link
Member

@Reinmar Reinmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented – there are concerns about browser compatibility. Also, we need to be able to test it on a manual test.

@mfreed7
Copy link

mfreed7 commented Nov 11, 2021

Hello, I just wanted to check in on this issue, and point out a new proposal for a Shadow DOM compatible Selection API. This proposal has been being discussed on WICG/webcomponents#79. In particular, this comment gives a quick TL;DR summary of the proposal.

In particular, I'd be curious to hear the opinions of the CKEditor maintainers as to whether this proposal will suit your needs:

  1. Is the proposal understandable to you?

  2. Does the proposed getComposedRange() API work for your use case?

  3. In particular, do the two parameters, selectionRoot and shadowRoots make sense? And would they allow you to easily make use of cross-scope selection information?

  4. Do the changes to the "setter" APIs like setBaseAndExtent() seem like they'll work for your use case?

  5. Do you have any need to get selection information "anywhere on the page", i.e. within any shadow root? This would likely require another parameter to make this easy, if so.

  6. Do you have any other concerns or feedback?

I'd love to hear your feedback either here in this issue, or (better) on the Selection API thread. These changes are still very much in development, so your feedback can have an impact. Thanks!

@wimleers
Copy link

FYI: Drupal would also like to see this happen.

@pomek
Copy link
Member

pomek commented Feb 9, 2022

The PR is a little outdated as the last update was made over a year ago, but I wanted to verify whether it resolves my case. After merging the latest #master, the selection in Shadow DOM and the editor's selection are not synchronized.

Perhaps I failed when merging changes…

@Inviz
Copy link
Contributor

Inviz commented Oct 15, 2022

I was a bit disappointed to learn that CKEditor does not handle running in the web component. thanks to this PR though i shimmed it like this:

var orig = document.defaultView.getSelection;
document.defaultView.getSelection = function() {
  if (document.activeElement.shadowRoot) return document.activeElement.shadowRoot.getSelection(); 
  return orig.apply(this, arguments);
}

Obviously it's not great, but at least i didnt have to hack the source code. You may need to have a strictier check for safety, e.g. to check the tag name of activeElement.

But because of lacking browser support for this, it looks like i'm better of implementing edtiable field as a slot instead

@CKEditorBot
Copy link
Collaborator

There has been no activity on this PR for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the contribution, leave a comment or reaction under this PR.

@CKEditorBot
Copy link
Collaborator

We've closed your PR due to inactivity over the last year. While time has passed, the core of your contribution might still be relevant. If you're able, consider reopening a similar PR.

@CKEditorBot CKEditorBot added resolution:expired This issue was closed due to lack of feedback. and removed status:stale labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:expired This issue was closed due to lack of feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants