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

RESOLVED FIXED in Firefox 60

Status

()

RESOLVED FIXED
5 years ago
13 days ago

People

(Reporter: azasypkin, Assigned: smaug)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla60
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

Posted 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)
(Assignee)

Comment 5

3 years ago
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)
(Assignee)

Updated

3 years ago
Blocks: 1233851
(Assignee)

Comment 6

3 years ago
Posted 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
(Assignee)

Updated

a year ago
Blocks: 1205323
(Assignee)

Comment 9

a year ago
Posted file testcase
Attachment #8488994 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Blocks: 1409959
(Assignee)

Updated

a year ago
Blocks: 1421833
(Assignee)

Updated

a year ago
Duplicate of this bug: 1233851
(Assignee)

Updated

a year ago
Depends on: 1441647
(Assignee)

Comment 11

a year ago
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)
(Assignee)

Comment 12

a year ago
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+
(Assignee)

Comment 15

a year ago
Posted patch nit fixed (obsolete) — Splinter Review
(Assignee)

Comment 16

a year ago
Posted patch rebasedSplinter Review
Attachment #8956287 - Attachment is obsolete: true

Comment 17

a year ago
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
Last Resolved: a year ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
QA Whiteboard: [good first verify]
Depends on: 1497480
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.