1752694 - slot fallback node remains visible

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[orstavik77]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0

Steps to reproduce:

<my-details-frame>
  <span summary>one</span>
  <div>sunshine</div>
</my-details-frame>

<script>

  const myDetailsHTML = `
<style>
  slot#content{display: none;}
  b {display: inline-block;}
  :host([open]) slot#content{display: block;}
  :host([open]) b {transform: rotate(90deg);}
</style>
<span><b>&#x25BA;</b><slot id="summary" summary>***hubba**</slot></span>
<slot id="content">!!!bubba!!!</slot>`;

  class MyDetails extends HTMLElement {
    constructor() {
      super();
      console.log(this.tagName, "constructor");
      this.attachShadow({mode: 'open', slotAssignment: "manual"});
      this.shadowRoot.innerHTML = myDetailsHTML;
      this.shadowRoot.querySelector('span').addEventListener('click', e =>
        this.hasAttribute('open') ? this.removeAttribute('open') : this.setAttribute('open', ''));
      setTimeout(()=>this.childChangedCallback());
    }

    childChangedCallback() {
      let slottables = [...this.childNodes];
      slottables = slottables.filter(n => !(n instanceof Comment)); //annoying, Comments should be slottable.
      const summaryPosition = slottables.findIndex(n => n.hasAttribute && n.hasAttribute("summary"));
      let summary;
      if (summaryPosition >= 0)
        summary = slottables.splice(summaryPosition, 1)[0];
      this.shadowRoot.getElementById("summary").assign(summary);
      this.shadowRoot.getElementById("content").assign(...slottables);
    }
  }

  class MyDetailsFrame extends HTMLElement {
    constructor() {
      super();
      this.attachShadow({mode: 'open', slotAssignment: 'manual'});
      this.shadowRoot.innerHTML = `
<!--      <my-details open style="border: 2px solid green; display: block;">-->
      <my-details style="border: 2px solid green; display: block;">
        <slot summary id="summary">***hubba**</slot>
        <slot id="content">!!!bubba!!!</slot>
      </my-details>
`;
      setTimeout(()=>this.childChangedCallback());
    }

    childChangedCallback() {
      let slottables = [...this.childNodes];
      slottables = slottables.filter(n => !(n instanceof Comment)); //annoying, Comments should be slottable.
      const summaryPosition = slottables.findIndex(n => n.hasAttribute && n.hasAttribute("summary"));
      let summary;
      if (summaryPosition >= 0)
        summary = slottables.splice(summaryPosition, 1)[0];
      this.shadowRoot.getElementById("summary").assign(summary);
      this.shadowRoot.getElementById("content").assign(...slottables);
    }
  }

  customElements.define("my-details", MyDetails);
  customElements.define("my-details-frame", MyDetailsFrame);
</script>

<script>
  setTimeout(function () {
    const frame_details = document.querySelector("my-details-frame");
    const inner_details = frame_details.shadowRoot.children[0];
    frame_details.insertAdjacentHTML('afterbegin', '<div>hello</div><span summary>two</span>');

    Promise.resolve().then(()=>{
      frame_details.childChangedCallback();
      inner_details.childChangedCallback();
    });
  }, 500);
</script>

Actual results:

The view shows 'twoone' after 500ms.

Expected results:

The view should show 'one' for 0-500ms, and then only 'two' after 500ms.

But. When you click on the summary which reveals the new visible/slotted position of 'one', then 'one' is removed from the summary slot (as it should have been earlier on). It looks like the underlying view isn't alerted that the 'one' element that was previously slotted under <slot summary> is now removed, when that elements new position is display none somewhere else.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

FWIW, I got the result ***hubba**two on Chrome.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Seems like an issue with manual slotting. Sean perhaps you can take a look?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: test-case from reporter.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: More reduced test-case.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ah, found it...

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1752694 - Invalidate layout before moving out of mAssignedNodes. r=smaug,sefeng

Otherwise we won't clear frames / style data from the assigned nodes.
I'm surprised there's no existing test catching this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b4089d4c9d8
Invalidate layout before moving out of mAssignedNodes. r=sefeng

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/32687 for changes under testing/web-platform/tests

Comment 11

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4b4089d4c9d8

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot