Import editing host of definition from the execCommand spec

[janiceshiu]

I am continuing to import relevant definitions from the execCommand spec. For further context, please see this HTML spec PR.

cc @marcoscaceres

---

💥 Error: Wattsi server error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 8:00 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.

🔗 Related URL

Parsing MDN data...
Parsing...

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

[domenic]

Hmm. This one seems less obviously-good for importing into HTML, but if the plan is to import both editable and editing host of, then I think it works OK. I'd prefer to do that in a single pull request, though, if that's OK with you?

[janiceshiu]

@domenic execCommand's definition for editable is here.

I added it into the html spec in this commit and rewrote it into a more algorithmic manner in this commit.

@marcoscaceres and I rewrote the definition. Please help us check the rewritten version. Thank you.

[janiceshiu]

@domenic In execCommand's definition for editable, Node references DOM's #concept-node. However, in the HTML spec, <span>node</span> references DOM's #interface-node.

Here is an example of <span>node</span being used in the HTML spec.

I am not sure about 2 things.

1. In this case, which DOM node definition should Node refer to?
2. Should we keep the execCommand definition's reference to #concept-node and add the #concept-node definition to the dependencies section, I need to have a unique ID for its data-x.

Please let me know what you think is best.

Thank you.

[domenic]

I don't really understand this step. I think we should just remove it, since you already have "a node is editable if". The execCommand spec's "something is editable if" is very unusual.

Apart from that, generally in HTML we seem not to cross-reference the word "node".

[janiceshiu]

I see. Rereading over my definitions, I notice that for both editing host of and editable, the definitions mention a node <var>node</var>:.

My understanding is that <var>node</var> is a variable with the name node. The standalone node seems to imply that the <var>node</var> is of type Node. However, it doesn't seem very clear because the standalone node does not link to any definition of type Node. Perhaps we could link to a suitable definition to make it very explicit to anyone reading these definitions?

As you noted elsewhere, that would also remove the need to write something like If <p>If <var>node</var> is not an instance of Node, return false.</p>

[domenic]

Although I generally agree with you, so far in the HTML Standard there are some concepts which are used so widely that so far we haven't linked to them. "node" is one of those, as is "element". I would support an effort to cross-link all of those (although we might want to check with other editors first, in case they think it would be too noisy). But I think that would be a separate project, and for now we should just use "node" with no cross-linking.

[janiceshiu]

Ah, thanks for explaining! While applying your fixes, I also realised that Element isn't cross-linked and was going to ask about that, so thank you for pre-empting my question.

[domenic]

I don't really understand this step. I think we should just remove it, since you already have "a node is editable if". The execCommand spec's "something is editable if" is very unusual.

Apart from that, generally in HTML we seem not to cross-reference the word "node".

[domenic]

This algorithm doesn't seem quite correct. For example editing hosts are not editable according to the execCommand spec: "Something is editable if ... it is not an editing host ...". But the second step of this algorithm returns false.

Also this algorithm omits the HTML element/svg/math element bits.

I think you want something either of the form:

A node node is editable if it meets all of the following criteria:

• node is not an editing host;
• node's contenteditable attribute is not in the false state;
• ...; and
• ...

or of the form

A node node is editable if the following algorithm returns true:

1. If node is an editing host, then return false.
2. If node's contenteditable attribute is in the false state, then return false.
3. If ..., then return false.
4. If ..., then return false.
5. Return true.

[janiceshiu]

@domenic Makes sense, thank you! Also thanks for catching the accidentally omitted parts.

I rewrote the algorithm to follow your A node node is editable if the following algorithm returns true: format.

This is how I broke it down.

Something is editable if it is a node;

<p>A node <var>node</var> is <dfn data-export="">editable</dfn> if the following algorithm returns
  true:</p>

it is not an editing host;

<li>If <var>node</var> is an <span>editing host</span>, then return false.</li>

it does not have a contenteditable attribute set to the false state;

<li>If <var>node</var>'s <code data-x="attr-contenteditable">contenteditable</code> attribute is
   set to the <i>false</i> state, then return false.</li>

its parent is an editing host or editable;

   <li>If <var>node</var>'s parent is neither an <span>editing host</span> nor
   <span>editable</span>, then return false.

and either it is an HTML element, or it is an svg or math element, or it is not an Element and its parent is an HTML element.

   <li>If <var>node</var> is not an Element and its parent is not an HTML element, then return
   false</li>

   <li>If <var>node</var> is neither an HTML element, an SVG element, nor a math element, then
   return false.</li>

   <li>Return true.</li>

[janiceshiu]

there is also a note attached to execCommand's definition of editable.

An editable node cannot be a document or DocumentFragment, its parent cannot be null, and it must descend from either an Element or a document.

Is it correct to assume that the algorithm should take this into account as well when returning true or false?

[domenic]

I think the note is explaining consequences of the current definition, which will also be true with the algorithm you're writing. So I don't think we need to include it. In particular:

• An editable node cannot be a document: true, because documents are not elements, and they have no parents, so their parents are not in the HTML namespace.
• An editable node cannot be a document fragment: same reasoning.
• Its parent cannot be null: true, after we one of the fixes I suggested
• It must descend from either an Element or a document: it's not clear what "descend" means, but maybe it means that it must have an element or document ancestor? If so, that's true, if you trace through the definitions.

Sometimes we include non-normative notes like this "summarizing" consequences of the more formal definition. But I think that's not necessary for the algorithm we're creating, because the algorithm version is already pretty clear.

[janiceshiu]

All updated as per your latest change requests! Thank you so much for all your explanations.

[janiceshiu]

Please hold off on reviewing this - am writing up Javascript algorithms for both editing host and editable definitions as they currently stand after the latest edits pushed. I'm also writing tests for these algorithms so that @marcoscaceres and I can confirm that the rewritten algorithm performs as expected.

Thank you!

[domenic]

Sounds good; just ping this thread again when you're ready for a new review!

[janiceshiu]

Hi @domenic!

I've written JS implementations for the editable and editing host algorithms here. I've also written tests for them. From that work, @marcoscaceres and I noticed the following things:
1.If node is not an Element, and node's parent is an element in the HTML namespace, then return true. had a bug.

• Document, DocumentType, DocumentFragment, Attr, CharacterData, and Element all inherit from Node.
• Thus, if <var>node</var> is an instanceof Node but is not an instanceof an Element, it is a Document, DocumentType, DocumentFragment, Attr, or CharacterData. Of all of these, only Attr has a 'parent' of sorts in ownerElement. The others do not and thus cannot be editable.
• To solve this, we changed the algorithm to first check that <var>node</var> is an Element or an Attr before proceeding with other checks.

2. If <var>node</var>'s parent is not very clear in terms of what node's parent is referring to.
   *Attr has an ownerElement. However, Element has a parentElement. The older algorithm said

• To solve this, we added a line to specifically define <var>parent</var> first to make the later parts of the algorithm clearer.

3. I am not sure whether to refer to attribute nodes by their class name Attr or as attributes as per the DOM spec definition. Please let me know. Thanks!

Ready for your review again. ^^

Note
While implementing the editing host algorithm, I got confused by

a <span data-x="concept-tree-child">child</span> <span data-x="HTML 
elements">HTML element</span> of a <code>Document</code> with <code 
data-x="dom-document-designMode">designMode</code> enabled.</p>

because the spec text seemed to imply that the node being tested should be a child of Document. That is, node.parentElement should be a Document. Am wondering whether something like a child HTML element whose Document has designMode enabled might be clearer. Having said that, if there are any changes to editing host's definition, I'll make that in a different PR.

[domenic]

Attrs should not be editable or editing hosts, so I think we should remove all that complexity.

because the spec text seemed to imply that the node being tested should be a child of Document.

I think that's correct. That condition is only met for children of Document, from what I understand.

[domenic]

This needs to say "If node is an element and node's contenteditable attribute...", since otherwise you're inspecting the attribute of an arbitrary node, which doesn't really work. Only elements have attributes.

[domenic]

These two steps don't seem quite right as a translation of

and either it is an HTML element, or it is an svg or math element, or it is not an Element and its parent is an HTML element.

because something which is not an element and whose parent is a HTML element, will return false now, because "If node is neither an HTML element, an SVG element, nor a math element" will trigger.

I think it might be worth flipping things a bit now that we've gotten to this point:

• If node is an element in the HTML namespace, SVG namespace, or MathML namespace, then return true.
• If node is not an element, and node's parent is an element in the HTML namespace, then return true.
• Return false.

(cross-link the three namespaces).

[domenic]

Although I generally agree with you, so far in the HTML Standard there are some concepts which are used so widely that so far we haven't linked to them. "node" is one of those, as is "element". I would support an effort to cross-link all of those (although we might want to check with other editors first, in case they think it would be too noisy). But I think that would be a separate project, and for now we should just use "node" with no cross-linking.

[domenic]

I guess we should insert a step before this one, which says "If node's parent is null, then return false". Otherwise we are checking whether null is an editing host or editable, and technically those algorithms do not take null as an allowed value.

[domenic]

I think the note is explaining consequences of the current definition, which will also be true with the algorithm you're writing. So I don't think we need to include it. In particular:

• An editable node cannot be a document: true, because documents are not elements, and they have no parents, so their parents are not in the HTML namespace.
• An editable node cannot be a document fragment: same reasoning.
• Its parent cannot be null: true, after we one of the fixes I suggested
• It must descend from either an Element or a document: it's not clear what "descend" means, but maybe it means that it must have an element or document ancestor? If so, that's true, if you trace through the definitions.

Sometimes we include non-normative notes like this "summarizing" consequences of the more formal definition. But I think that's not necessary for the algorithm we're creating, because the algorithm version is already pretty clear.

[marcoscaceres]

We might be able to change the tests to check .isContentEditable, which means we should be able to move the tests to WTP 🤞. CC @johanneswilm, as it may be of interest.