1355893 - Wrong element is selected in inspector after deleting an element, if whitespace text node is present

Status: NEW

(DevTools :: Inspector, defect, P2)

Description

[684sigma]

I decided to check how Bug 1345529 was fixed, and noticed another bug in Nightly 55 (2017-04-11). It happens in earlier versions in another form. It all started after Bug 1304685.
There're two ways to reproduce the bug:

First:
1. Navigate to this url - data:text/html,<div><p><a>test1</a> <b>test2</b></p></div>
2. Open inspector, delete node <a>

Result:
Before Bug 1304685 - <b> is selected
After Bug 1304685 - no node is selected (whitespace text node is selected and then removed from inspector)
After Bug 1345529 - parent node of <a> is selected (<p>)

Expected: <b> should be selected



Second:
1. Navigate to this url - data:text/html,<div><p><a>test1</a> <b>test2</b></p></div>
2. Open inspector, delete node <b>

Result:
Before Bug 1304685 - <a> is selected
After Bug 1304685 - child nodes of parent element are removed from inspector (Bug 1345529)
After Bug 1345529 - no element is selected (whitespace text node is selected and then removed from inspector)

Expected: <a> should be selected

Comment 1

[684sigma]

I tracked down each change from comment #0, and mozregression-gui pointed to Bug 1304685 and Bug 1345529


https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=687572ea20cb84f1842ad00dcc8ca1712868a361&tochange=fe1d7c28668f0eb07c2f91bd0b3b0fa635d82387
->
1304685 – Help debug spacing issues caused by whitespace text nodes
https://bugzilla.mozilla.org/show_bug.cgi?id=1304685


https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e2bcdd1501405f8bc8e5841a06088fd3fc179878&tochange=d508a91f09e9de268da1b7831c823c6a42f16526
->
1345529 – [Regression] Partially elements may lost when remove element in Inspector
https://bugzilla.mozilla.org/show_bug.cgi?id=1345529


(Mozregression-gui 0.9.7 is broken due to Bug 1354451, consider using working 0.9.6 version)

Comment 2

[Liz Henry (:lizzard)]

Too late for a fix in 53.

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

With your latest fix is this fixed or not?

Comment 4

[Julian Descottes [:jdescottes]]

No it's not fixed. The state is still:

> After Bug 1345529 - no element is selected (whitespace text node is selected and then removed from inspector)

Comment 5

[Patrick Brosset <:pbro>]

DevTools bug triage (filter on CLIMBING SHOES).

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

The issue comes from the removeNode function in https://searchfox.org/mozilla-central/rev/c5ed4a89fd7a8c878df552e5d53fe50e0088c15b/devtools/client/fronts/walker.js#333-340

async removeNode(node) {
  const previousSibling = await this.previousSibling(node);
  const nextSibling = await super.removeNode(node);
  return {
    previousSibling,
    nextSibling,
  };
}

So here, we're retrieving the previous and next siblings of the node we're going to remove, to select one or the other.
The issue is that when those siblings are whitespace text nodes, removing the node will make the whitespace text node to be selected, but also, now the whitespace text node is going to be skipped, i.e. removed from the markup view, so we end up with no selected node at all.

The fix isn't straightforward as we'd need to know before removing the node if it will impact the visibility of the siblings, and we don't have an easy way to do that.

Another solution would be to better handle re-selecting a sibling node when the selected whitespace text node is removed