[Webkit-unassigned] [Bug 188279] New: Range APIs do not construct / move trees in tree order (observable by custom elements)

Thu Aug 2 15:15:39 PDT 2018

https://bugs.webkit.org/show_bug.cgi?id=188279

            Bug ID: 188279
           Summary: Range APIs do not construct / move trees in tree order
                    (observable by custom elements)
           Product: WebKit
           Version: Safari 11
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: HTML DOM
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: bicknellr at google.com
                CC: cdumez at apple.com

Created attachment 346421

  --> https://bugs.webkit.org/attachment.cgi?id=346421&action=review

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180802/d99acb58/attachment-0001.html>