Closed Bug 1066965 Opened 6 years ago Closed 2 years ago

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

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: azasypkin, Assigned: smaug)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files, 3 obsolete files)

Attached file shadow-dom-content-editable.html (obsolete) —
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...
Blocks: 1067228
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 ?
Flags: needinfo?(overholt)
*prioritize :)
As a workaround you could use a custom-element without shadow-dom. But agree, this should be fixed. William or Smaug to the rescue :)
William/Olli: what do you think?
Flags: needinfo?(wchen)
Flags: needinfo?(overholt)
Flags: needinfo?(bugs)
I've been looking at this a bit today. Still having some wrong kind of is-in-(Un)composed check somewhere.
Assignee: nobody → bugs
Flags: needinfo?(wchen)
Flags: needinfo?(bugs)
Blocks: 1233851
Attached patch v1 (obsolete) — Splinter Review
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
Attachment #8700187 - Flags: review?(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
Attachment #8700187 - Flags: review?(wchen)
Duplicate of this bug: 1421831
Attached file testcase
Attachment #8488994 - Attachment is obsolete: true
Blocks: 1409959
Blocks: 1421833
Duplicate of this bug: 1233851
Depends on: 1441647
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.
Attachment #8700187 - Attachment is obsolete: true
Attachment #8954529 - Flags: review?(mrbkap)
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 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?
Attachment #8954529 - Flags: review?(mrbkap) → review+
Attached patch nit fixed (obsolete) — Splinter Review
Attached patch rebasedSplinter Review
Attachment #8956287 - Attachment is obsolete: true
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5934e66f2c77
make contentEditable and spellchecking to work in ShadowDOM, r=mrbkap
https://hg.mozilla.org/mozilla-central/rev/5934e66f2c77
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
QA Whiteboard: [good first verify]
Component: DOM → DOM: Core & HTML

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

You need to log in before you can comment on or make changes to this bug.