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

DOM "insertion steps" #2771

Closed
cvrebert opened this issue Jun 17, 2017 · 15 comments · Fixed by #7712
Closed

DOM "insertion steps" #2771

cvrebert opened this issue Jun 17, 2017 · 15 comments · Fixed by #7712
Labels
clarification Standard could be clearer good first issue Ideal for someone new to a WHATWG standard or software project

Comments

@cvrebert
Copy link
Member

https://html.spec.whatwg.org/#dom-trees

A node A is inserted into a node B when the insertion steps are invoked with

Per https://dom.spec.whatwg.org/#concept-node-insert-ext , "insertion steps" are supposed to be algorithms, but the HTML spec does not define any explicit insertion steps algorithm(s).
It just says "A <conceptual event> [occurs] when the insertion steps are invoked with <conditions>" a few times, with which set of insertion steps "the insertion steps" refers to being unclear.

I would instead expect something more like:

For all nodes, the insertion steps, given a node node, are to run <good algo name here> for node.

To <good algo name here>, given a node node, run these steps:

  1. If <conditions>, then [...]
  2. Etc...
@cvrebert
Copy link
Member Author

@annevk Your thoughts, as DOM editor?

@annevk annevk added the clarification Standard could be clearer label Jun 28, 2017
@annevk
Copy link
Member

annevk commented Jun 28, 2017

I do think that makes sense. We just haven't gone down that rabbit hole yet due to the amount of effort required and not everything being set up as clear from the beginning.

@domenic
Copy link
Member

domenic commented Jun 28, 2017

I'll tag this as "good first bug", although it's on the more difficult side. Explicitly, what you'd want to do is:

  • Find every usage of "inserted into" for nodes. This can be accomplished by clicking on the term "A node A is inserted" at https://html.spec.whatwg.org/#nodes-are-inserted.
  • Replace each with an explicit set of "insertion steps" for that element, usually defined as an algorithm (although e.g. the img element case is a bit trickier).

People can feel free to work on individual instances here, or e.g. do the ones that look easy and ask questions about the harder ones.

@domenic domenic added the good first issue Ideal for someone new to a WHATWG standard or software project label Jun 28, 2017
@domenic
Copy link
Member

domenic commented Jun 28, 2017

I guess ideally we'd also do this for "node A is removed", "node is inserted into a document", "node is removed from a document", "becomes connected", and "becomes disconnected". Hmm. Now I am not as sure this is worthwhile; maybe it would just obscure things.

@Adrija1999
Copy link

Hi, I am a newbie and I've finished a course on html and it's been a week. As this is a "good first issue" I would like to try resolve this. It would be great if someone can just give me a basic introduction on what to do.

@sylph01
Copy link
Contributor

sylph01 commented Mar 7, 2022

Hi, I am thinking of contributing to this issue. I would like some confirmation/clarification before I actually send my patch in.

As far as I understand, this issue's goal is to rewrite every instance of "is inserted into" with specific insertion steps, is that correct?
So, a quick example of this for https://html.spec.whatwg.org/#the-source-element:nodes-are-inserted would be writing down the content for the insertion step algorithm, which would be something like the following in pseudocode (but actually in words):

function insertionStepForSource(sourceElement, mediaElement) {
  if (mediaElement.src === undefined && mediaElement.networkState === NETWORK_EMPTY) {
	  resourceSelectionAlgorithm(mediaElement);
	}
}

and changing the wording to something like:

If a source element is inserted as a child of a media element, the insertion steps are to run the following:
(algorithm written down here)

If this is the way to go, I will start with the more trivial ones, then go on to trying the less obvious ones afterwards.

Thank you!

@domenic
Copy link
Member

domenic commented Mar 7, 2022

Hmm. I think you are on the right track given how the issue is described. But, I'm revisiting this issue five years later, and I'm no longer sure it's a good idea.

My understanding of how DOM works is that it says specifications may define insertion steps, which take insertedNode. So basically it expects one set of "insertion steps" for all of the HTML Standard, which contains a lot of if statements. Something like:

The HTML Standard defines the following [insertion steps], given insertedNode:

  1. If insertedNode is a source element and is a child of a media element that has no src attribute and..., then: invoke the media element's resource selection algorithm.
  2. If insertedNode is a source element and its next sibling is an img element and their parent is a picture element, then, count this as a relevant mutation for the img element.
  3. If insertedNode is a form-associated element or the ancestor of a form-associated element, then: ...
    ...

This seems kind of bad!!

An alternate way of doing it would be for HTML to define many insertion steps. Something like:

The following [insertion steps], given insertedNode, apply:

  1. If insertedNode is a source element and is a child of a media element that has no src attribute and..., then: invoke the media element's resource selection algorithm.

and then do that for every element.

But my take is that HTML is already defining a single set of insertion steps, just in a distributed fashion. And it's doing so pretty rigorously already. Look at the definition of "a node is inserted":

A node A is inserted into a node B when the insertion steps are invoked with A as the argument and A's new parent is B.

I take this as basically synonymous with:

The HTML Standard defines the following [insertion steps], given insertedNode.

  1. Let parent be insertedNode's parent.
  2. Run any steps that this standard marks as taking place when insertedNode [is inserted into] parent.

so I think the current specification is already pretty good!

@annevk, what do you think? @sylph01, sorry if this ends up resulting in some wasted time of yours investigating!

@sylph01
Copy link
Contributor

sylph01 commented Mar 8, 2022

sorry if this ends up resulting in some wasted time of yours investigating!

No problem!

If anything needs to be changed here, I might suggest that writing out "Run any steps that this standard marks as taking place when insertedNode [is inserted into] parent" explicitly would clarify what "insertion steps" mean in HTML Standard's context and also tells the reader that "insertion steps" are defined at each element in a distributed fashion.
Maybe we can add in the definition of "a node is inserted" something like:

Specific insertion steps for each element type are defined under each element type's standard as steps that take place when insertedNode is inserted into its parent.

@annevk
Copy link
Member

annevk commented Mar 10, 2022

I haven't checked if this is a problem in practice, but a theoretical problem here is the lack of defined ordering. That's also a problem with the extension point. That usually argues for a single algorithm or a more coordinated approach.

@domenic
Copy link
Member

domenic commented Mar 14, 2022

@annevk and I discussed this a bit more in chat and we think the best path forward is to do something similar to what the current spec is doing, but more rigorous and well-grounded. This is similar to my previous comment but with a different style.

So I think the plan would be something like the following:

  • Introduce HTML element insertion steps concept for a local name name. This is a new concept which will gradually replace "node A is inserted".

  • Explicitly define the HTML Standard's insertion steps, like I did above. Something like "The HTML Standard defines the following [insertion steps], given insertedNode: 1. If insertedNode's [namespace] is the [HTML namespace], and this standard defines [HTML element insertion steps] for insertedNode's [local name], then run the corresponding [HTML element insertion steps] given insertedNode.

  • Replace all instances of "element is inserted" with "HTML element insertion steps". E.g. the source element one would become something like "The source HTML element insertion steps, given insertedNode, are: 1. If insertedNode's parent is a media element that has no src attribute and whose..., then invoke that media element's resource selection algorithm."

Does that make sense? If so, please feel free to start on a pull request, probably with just one or two replacements for now since setting up the initial definition infrastructure will be the hardest part that we want to review early.

@sylph01
Copy link
Contributor

sylph01 commented Mar 16, 2022

Yes, I think this approach makes sense. From reading the current wording, I will first prepare a pull request that does the following:

  • After "The term empty," and before "A node A is inserted':
    • Introduce "HTML element insertion step" for a local name
      • Announce that the HTML Standard may define HTML element insertion steps for a given local name
    • Using that concept, define what HTML Standard's insertion steps are
    • With this, the following paragraphs that depend on the phrase [insertion steps] can use the definition of HTML Standard's insertion steps. This may also mean that "node A is inserted" and the like can be used as-is without replacing.

I also noticed that this might need a follow-up where the same thing would be done for [removing steps] and [children changed steps] if they have specific instances in the HTML Standard. I will try to finish the insertion steps first, but if there's a need for this change, I might do it in the same PR as well.

@sylph01
Copy link
Contributor

sylph01 commented Mar 16, 2022

Created a work-in-progress PR at #7712. This PR currently has the fixes mentioned above.

@domenic
Copy link
Member

domenic commented Mar 21, 2022

#7712 is almost ready. However it turns out it is just the first step as there are a number of dependent definitions. Here is a checklist:

  • Make rigorous "node is inserted" / "node is removed": Clarify usage of "insertion steps" in the HTML Standard #7712.
  • Make rigorous "node is inserted into a document" / "node is removed from a document".
  • Make rigorous "becomes connected" / "becomes disconnected".
  • Make rigorous "becomes browsing-context connected" / "becomes browsing-context disconnected".

We should do those in separate PRs. I think the foundation of #7712 will make this much easier, as we can extend the HTML Standard insertion steps to call out to various algorithms. However there are some tricky subtleties that make it probably not a good-first-issue anymore, such as:

  • meta elements like to define multiple "node is inserted into a document" steps, in different sections. Should we consolidate them? Should we allow multiple "node is inserted" steps per element? (Probably we should consolidate them, which involves its own mini internal dispatch.)
  • autofocus can cause any element to react to "inserted into a document". So probably we need to add autofocus processing to the general HTML Standard insertion steps?
  • There are a lot of non-normative references to "node is inserted into a document". We would need to think of ways to update them. This could be as simple as just removing the links, but it requires some judgment.
  • Many of the references are similar to the "relevant mutations" examples which @sylph01 has already converted in Clarify usage of "insertion steps" in the HTML Standard #7712. Those are a bit trickier than the others, although we already have a good example of how to convert them.

Anyway, @sylph01 or any other contributor is welcome to continue working on this project after #7712. But I also want to acknowledge that that will be a good intermediate stopping point, and the work will get trickier after that.

domenic pushed a commit that referenced this issue Mar 28, 2022
Part of #2771. This removes the concepts of "node is inserted" and "node is removed" in favor of explicit HTML Standard insertion/removing steps, which are of the form the DOM Standard wants us to use. These HTML Standard insertion/removing steps, in turn, call out to element definition-specific "HTML element insertion/removing steps".

This does not complete #2771, as there remain several less-rigorous definitions, outlined in #2771 (comment). But it gives us the right scaffolding and prevents any further usage of the node is inserted/removed definitions.
@domenic domenic reopened this Mar 28, 2022
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
Part of whatwg#2771. This removes the concepts of "node is inserted" and "node is removed" in favor of explicit HTML Standard insertion/removing steps, which are of the form the DOM Standard wants us to use. These HTML Standard insertion/removing steps, in turn, call out to element definition-specific "HTML element insertion/removing steps".

This does not complete whatwg#2771, as there remain several less-rigorous definitions, outlined in whatwg#2771 (comment). But it gives us the right scaffolding and prevents any further usage of the node is inserted/removed definitions.
@dSalieri

This comment was marked as off-topic.

@domenic
Copy link
Member

domenic commented May 8, 2024

Moving to #10331.

@domenic domenic closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer good first issue Ideal for someone new to a WHATWG standard or software project
Development

Successfully merging a pull request may close this issue.

6 participants