Bug 188279 - Range APIs do not construct / move trees in tree order (observable by custom elements)
Summary: Range APIs do not construct / move trees in tree order (observable by custom ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari 11
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL: https://wpt.fyi/results/custom-elemen...
Keywords: InRadar
Depends on:
Blocks: 154907
  Show dependency treegraph
 
Reported: 2018-08-02 15:15 PDT by Russell Bicknell
Modified: 2023-02-07 22:35 PST (History)
6 users (show)

See Also:


Attachments
The test page mentioned in the bug. (5.19 KB, text/html)
2018-08-02 15:15 PDT, Russell Bicknell
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Russell Bicknell 2018-08-02 15:15:39 PDT
Created attachment 346421 [details]
The test page mentioned in the bug.

# Details of interop issue

  Chrome, Safari, and Firefox all appear to have the similar issues. Chrome and Safari sometimes show the same ordering; Firefox is different. None appears to follow the spec.

# Steps to reproduce the problem:

  1. Create a deep and wide tree of many custom elements with defined constructors, `connectedCallback`, and `disconnectedCallback`.
  2. Create a range that such that the boundaries of the range are deep enough in the tree to cause many of the custom elements to be only partially contained within the range.
  3. Call the range's `cloneContents` or `extractContents`.

  (Specific cases listed below.)

# What went wrong?

  Nodes in the cloned / extracted fragment are not created or moved in tree order:

  1. During `extractContents`, nodes that are partially contained in the range are cloned into the created fragment, rather than being moved out of the original range (as expected). However, the order in which the clones happen is now observably incorrect through the order in which the custom element constructors are called.

    To see this using the attached page:

    - Uncheck all checkboxes.
    - Click the "create source" button.
    - Check the "#n" and "NEW" checkboxes.
    - Click the "extract" button.

    You'll get a tree that looks like this:

    - [#2 NEW]
        - [#1 NEW]
            - [#0 NEW] START.....)
            - [] (root-0-0-2)
        - [] (root-0-1)
            - [] (root-0-1-0)
            - [] (root-0-1-1)
            - [] (root-0-1-2)
    - [] (root-1)
    - [#5 NEW] (root-2)
        - [] (root-2-0)
            - [] (root-2-0-0)
            - [] (root-2-0-1)
            - [] (root-2-0-2)
        - [#4 NEW] (root-2-1)
            - [] (root-2-1-0)
            - [#3 NEW] (root-2-1-1.....END

    In this tree, the constructors for the elements that were cloned as part of `extractContents` are not called in tree order. In the spec section about extracting a range [1], step 14 describes how the 'first partially contained child' of the nearest containing ancestor of a range is handled. Particularly, it explicitly clones the partially contained child first and extracts the range inside of it next. This is also true of step 17, describing how the 'last partially contained child' is handled.

  2. During `extractContents`, the ordering of `disconnectedCallback` calls of custom elements  which are moved (because they are fully contained) are not in tree order.

    Using the attached page:

    - Uncheck all checkboxes.
    - Click the "create source" button.
    - Check the "NEW" and "Dn" checkboxes.
    - Click the "extract" button.

    You'll get a tree that looks like this:

    - [NEW]
        - [NEW]
            - [NEW] START.....)
            - [, D0] (root-0-0-2)
        - [, D1] (root-0-1)
            - [, D2] (root-0-1-0)
            - [, D3] (root-0-1-1)
            - [, D4] (root-0-1-2)
    - [, D10] (root-1)
    - [NEW] (root-2)
        - [, D6] (root-2-0)
            - [, D7] (root-2-0-0)
            - [, D8] (root-2-0-1)
            - [, D9] (root-2-0-2)
        - [NEW] (root-2-1)
            - [, D5] (root-2-1-0)
            - [NEW] (root-2-1-1.....END

    (Sorry about the leading commas, they're not relevant.)

    Here, the logs show us that the custom elements were not removed from their original positions in the correct order: the flattened segment "D10, NEW, D6, D7, D8, D9, NEW, D5" should be "D5, NEW, D6, D7, D8, D9, NEW, D10".

  3. During `cloneContents`, the newly created tree is not constructed in tree order.

    Using the attached page:

    - Check only the "#n" checkbox.
    - Click the "create source" button.
    - Click the "clone" button.

    You'll get a tree that looks like this:

    - [#23]
        - [#21]
            - [#20] START.....)
            - [#22] (root-0-0-2)
        - [#24] (root-0-1)
            - [#25] (root-0-1-0)
            - [#26] (root-0-1-1)
            - [#27] (root-0-1-2)
    - [#36] (root-1)
    - [#31] (root-2)
        - [#32] (root-2-0)
            - [#33] (root-2-0-0)
            - [#34] (root-2-0-1)
            - [#35] (root-2-0-2)
        - [#29] (root-2-1)
            - [#30] (root-2-1-0)
            - [#28] (root-2-1-1.....END

    All of the elements in the output tree are cloned. Like the steps for extracting a range [1], the steps for cloning a range [2] imply that the new tree should be constructed in tree order. So, the numbers in the new tree here should be in ascending order from top to bottom.

  [1]: https://dom.spec.whatwg.org/#concept-range-extract
  [2]: https://dom.spec.whatwg.org/#concept-range-clone

# Any other comments?

  The attached file is meant to let you see the ordering of construction / the callbacks. It defines a custom element that displays a small log of when its constructor and callbacks are called. (In brackets, at the start of the element.) You can toggle which things are logged using the checkboxes at the top of the page. Each logged event increments a global and prints its value so you can see the relative order of all of the things you've enabled.

  - "#n" shows when the element's constructor is run.
  - "NEW" shows that the element was constructed during the `cloneContents` or `extractContents` call.
  - "Cn" shows when the element's `connectedCallback` is run.
  - "Dn" shows when the element's `disconnectedCallback` is run.

  ... where 'n' is the step at which the event happened. For example, an element with the log "[#10, C11, D42, C60]" means "constructed at step 10, connectedCallback at step 11, disconnectedCallback at step 42, connectedCallback at step 60" and "[#49 NEW, C71]" means "constructed at step 49 during the range operation, connectedCallback at step 71".

  Really, there are only two things to do to use the page:
  1. Click the "create source" button to create the 'source' tree (the one in which the range's boundary points will be placed). A tree in a template will be copied into the blue dashed box (using innerHTML, rather than cloneNode, so that it's created by the parser).
  2. Click either the "extract" or "clone" button to extract / clone a range between the two nodes with class "start" and "end" (inclusive). The fragment is then appended to the orange dashed box.

  The reason there's an explicit step for creating the source tree is so that you can change the stuff that's logged when the tree is created.

  The elements have some text in them (e.g. "(root-2-0-1)") to show their position in the source tree and also to make it more clear where the boundary points of the range are in that node. None of the CSS is actually relevant to the repro.
Comment 1 Russell Bicknell 2018-08-02 15:18:36 PDT
Also, if you check the "#n" and "Cn" checkboxes and click "create source", you'll notice that all of the elements in the source tree were constructed before any of them are connected. I'll file a separate bug for this since it's not related to ranges.
Comment 2 Russell Bicknell 2018-08-02 15:21:00 PDT
Possibly related: https://bugs.webkit.org/show_bug.cgi?id=160667
Comment 4 Ryosuke Niwa 2018-08-04 22:04:52 PDT
This is presumably also observable via mutation observers since disconnection of a node is typically caused by a removal of the node, which is observable via mutations observers.
Comment 5 Ryosuke Niwa 2023-02-06 18:12:08 PST
Pull request: https://github.com/WebKit/WebKit/pull/9730
Comment 6 EWS 2023-02-07 17:02:55 PST
Committed 259987@main (fa5a06a66c96): <https://commits.webkit.org/259987@main>

Reviewed commits have been landed. Closing PR #9730 and removing active labels.
Comment 7 Radar WebKit Bug Importer 2023-02-07 17:03:22 PST
<rdar://problem/105154132>