1066965 - [WebComponents] Content editable elements don't work when in shadow dom

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Oleg Zasypkin [:azasypkin]]

Attachment: shadow-dom-content-editable.html

It's impossible to put focus inside contenteditable element to actually edit its content. Please, see attached test case.

Tested on the latest Nightly, works in Chrome...

Comment 1

[Julien Wajsberg [:julienw]]

Still happening in latest nightly.

Hey Andrew, this is a really painful issue in web component for us, as we'd like to embed our composer or our recipient panel in a web component, and these components use contenteditable elements.

Is it possible to priorize this ?

Comment 2

[Julien Wajsberg [:julienw]]

*prioritize :)

Comment 3

[Wilson Page [:wilsonpage]]

As a workaround you could use a custom-element without shadow-dom. But agree, this should be fixed. William or Smaug to the rescue :)

Comment 4

[Andrew Overholt [:overholt]]

William/Olli: what do you think?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I've been looking at this a bit today. Still having some wrong kind of is-in-(Un)composed check somewhere.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v1

Spellchecker doesn't work with this yet, but it is somewhat different work, so better to do it in a separate bug.


https://treeherder.mozilla.org/#/jobs?repo=try&revision=3baed0a85075

Comment 7

[William Chen [:wchen]]

Comment on attachment 8700187 [details] [diff] [review]
v1

Review of attachment 8700187 [details] [diff] [review]:
-----------------------------------------------------------------

Now that we support editable elements in shadow DOM, we also need to update the editable descendant count when BindToTree recurses for shadow DOM content [1] similar to how it's done for XBL [2].

[1] https://hg.mozilla.org/mozilla-central/file/388bdc46ba51ee31da8b8abe977e0ca38d117434/dom/base/Element.cpp#l1681
[2] https://hg.mozilla.org/mozilla-central/file/388bdc46ba51ee31da8b8abe977e0ca38d117434/dom/base/Element.cpp#l1623

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: shadow_contenteditable.diff

This was mostly just hunting composed doc usage, bug also using the right focus event target.

I'll need to figure out some tests for this, but that may require fixing some focus handling issues first.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/135215a3fa71673dfdc4e7d52d2cce459c7d8e1f
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=135215a3fa71673dfdc4e7d52d2cce459c7d8e1f
remote: recorded changegroup in replication log in 0.088s

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8954529 [details] [diff] [review]
shadow_contenteditable.diff

Review of attachment 8954529 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsINode.h
@@ +1011,5 @@
>      return mParent;
>    }
>  
> +  /**
> +   * This is similar to above, but in case 'this' is ShadowRoot, we return it's

Nit: its

@@ +1014,5 @@
> +  /**
> +   * This is similar to above, but in case 'this' is ShadowRoot, we return it's
> +   * host element.
> +   */
> +  nsINode* GetParentOrHostNode() const;

Is it worth rewriting nsContentUtils::ContentIsShadowIncludingDescendantOf to use this function?

::: editor/libeditor/HTMLEditRules.cpp
@@ +8739,5 @@
>  
>    nsINode* temp = selNode;
>  
>    // check that selNode is inside body
> +  //XXXsmaug this code is insane.

Maybe file a bug on replacing this with non-insane code?

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

filed https://bugzilla.mozilla.org/show_bug.cgi?id=1443363

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nit fixed

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Attachment: rebased

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5934e66f2c77
make contentEditable and spellchecking to work in ShadowDOM, r=mrbkap

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5934e66f2c77

Comment 19

[Cihad Paksoy]

Stil happening... Firefox 72.0.1 (64 bit) Window 10.