Similar to getFlexParent() we could improve performance if we create getGridParent() and make use of that in the grid highlighter.

https://drafts.csswg.org/css-grid/#grid-items defines how grid items are associated with grid containers.

@brad Using node.parentFlexElement gives us a fairly dramatic increase in performance so if there is something similar you could do with grid that would be awesome.

(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #2)

@brad Using node.parentFlexElement gives us a fairly dramatic increase in performance so if there is something similar you could do with grid that would be awesome.

Mind I ask why / how? Perhaps there's something to optimize on the platform code. Can I see how the code was before parentFlexElement?

Basically, I'm not opposed to add devtools-only calls, but it'd be nice to know that they're not working around Gecko bugs.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #2)

@brad Using node.parentFlexElement gives us a fairly dramatic increase in performance so if there is something similar you could do with grid that would be awesome.

Mind I ask why / how? Perhaps there's something to optimize on the platform code. Can I see how the code was before parentFlexElement?

Before parentFlexElement we had to:

1. Get the node (either a flex item or one of it's children)... we only want the parent if we have a flex item and not a child of a flex item.
2. Using a TreeWalker walk up each ancestor until we find one with e.g. display:flex... this is the container.
3. We then need to use getAsFlexContainer() to get the flex details and iterate over .getLines() and getItems() to check if the node is a flex item as opposed to a child of a flex item.

The problem is that the highlighter code is very hot as it needs to keep up with every reflow etc. and findFlexParentContainerForNode() was causing noticeable lag because we had to call it on every hovered node every reflow... parentFlexElement greatly improved this lag.

Of course, if there are issues with doing the same for grid (the JS is simpler than the flexbox code) we can stick with the current code.

function findFlexParentContainerForNode(node) {
  const doc = node.ownerDocument;
  const win = doc.defaultView;
  const treeWalker = doc.createTreeWalker(doc.body, NodeFilter.SHOW_ELEMENT);
  let currentNode = null;

  treeWalker.currentNode = node;

  try {
    while ((currentNode = treeWalker.parentNode())) {
      const displayType = win.getComputedStyle(currentNode).display;
      if (!displayType) {
        break;
      }

      if (displayType.includes("flex")) {
        if (isNodeAFlexItemInContainer(node, currentNode)) {
          return currentNode;
        }
      } else if (displayType === "contents") {
        // Continue walking up the tree since the parent node is a content element.
        continue;
      }

      break;
    }
  } catch (e) {
    // Getting the parentNode can fail when the supplied node is in shadow DOM.
  }

  return null;
}

function isNodeAFlexItemInContainer(supposedItem, container) {
  const doc = container.ownerDocument;
  const win = doc.defaultView;
  const containerDisplayType = win.getComputedStyle(container).display;

  if (containerDisplayType.includes("flex")) {
    const containerFlex = container.getAsFlexContainer();

    for (const line of containerFlex.getLines()) {
      for (const item of line.getItems()) {
        if (item.node === supposedItem) {
          return true;
        }
      }
    }
  }

  return false;
}

That seems exceedingly complicated? I'd expect something like this to get the same result and be way faster than that / not much slower than the native code, but I might be missing something:

function findFlexParentContainerForNode(node) {
  if (node.nodeType == node.ELEMENT_NODE) {
    let style = getComputedStyle(node);
    let display = style.display;
    if (!display || display == "none" || display == "contents")
      return null; // Doesn't generate a box, not a flex item.
    let position = style.position;
    if (position == "absolute" || position == "fixed" || style.cssFloat != "none")
      return null; // Out of flow, not a flex item.
  }
  if (node.nodeType != node.TEXT_NODE)
    return null;
  for (let p = node.flattenedTreeParentNode; p; p = p.flattenedTreeParentNode) {
    let style = getComputedStyle(p);
    let display = style.display;
    if (display.includes("flex") && p.getAsFlexContainer())
      return p; // It's a flex item!
    if (display != "contents")
      return null; // Not a flex item, for sure.
    // display: contents, walk to the parent
  }
  return null;
}

We then need to use getAsFlexContainer() to get the flex details and iterate over .getLines() and getItems() to check if the node is a flex item as opposed to a child of a flex item.

Why do you need to do this? Are you trying to catch absolutely positioned elements or such (in which case they are not a flex item, but they're not a child either)? That should be caught by the position / float check above.

Are you trying to catch anonymous flex items? My code would return the flex container for text in an anonymous flex item, but if you want that to return null you just need to return null if the input node is not an element. Anonymous flex items can only be wrapping text, not other elements.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

That seems exceedingly complicated? I'd expect something like this to get the same result and be way faster than that / not much slower than the native code, but I might be missing something:

function findFlexParentContainerForNode(node) {
  if (node.nodeType == node.ELEMENT_NODE) {
    let style = getComputedStyle(node);
    let display = style.display;
    if (!display || display == "none" || display == "contents")
      return null; // Doesn't generate a box, not a flex item.
    let position = style.position;
    if (position == "absolute" || position == "fixed" || style.cssFloat != "none")
      return null; // Out of flow, not a flex item.
  }
  if (node.nodeType != node.TEXT_NODE)
    return null;
  for (let p = node.flattenedTreeParentNode; p; p = p.flattenedTreeParentNode) {
    let style = getComputedStyle(p);
    let display = style.display;
    if (display.includes("flex") && p.getAsFlexContainer())
      return p; // It's a flex item!
    if (display != "contents")
      return null; // Not a flex item, for sure.
    // display: contents, walk to the parent
  }
  return null;
}

We then need to use getAsFlexContainer() to get the flex details and iterate over .getLines() and getItems() to check if the node is a flex item as opposed to a child of a flex item.

Why do you need to do this? Are you trying to catch absolutely positioned elements or such (in which case they are not a flex item, but they're not a child either)? That should be caught by the position / float check above.

We highlight a flex item differently than children of a flex item so that is why we need to differentiate.

We want to return the container for flex items but null for children of flex items, hence the complexity.

Are you trying to catch anonymous flex items? My code would return the flex container for text in an anonymous flex item, but if you want that to return null you just need to return null if the input node is not an element. Anonymous flex items can only be wrapping text, not other elements.

We do not need to worry about anonymous flex items, that is covered in a different part of our code.

How does my function not return null for children of flex items?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

How does my function not return null for children of flex items?

It does, but it also returns null for flex items.

This is more easily discussed with an example (attached). I have temporarily replaced p.getAsFlexContainer() with p.id === "container" so that we can run it inside a HTML file.

It gives the following output:

FAIL #flexitem1 is a flex item... container is null Simple flexbox.html:34:9
FAIL #flexitem2 is a flex item... container is null Simple flexbox.html:34:9
PASS h1 is not a flex item... it is a child of a flex item. Simple flexbox.html:32:9
PASS h2 is not a flex item... it is a child of a flex item.

So your method assumes that a node is not a flex item if it is a text node, which isn't true.

(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #9)

So your method assumes that a node is not a flex item if it is a text node, which isn't true.

Blerg, that's because I don't know how to code ;)

That's supposed to be an else if, so, if the node is not an element and not text, then it's not a flex item. Also, note that flattenedTreeParentNode is ChromeOnly, but for this test-case, parentNode is the same. So with those changes all those tests pass.

Since the logic is simple and the there's no indication that platform changes are required for performance, let's not add Platform API for this. Let's continue to implement this in JS, possibly using the more efficient JS template provided by Emilio (using Flex as an example) in comment 6.

The general consensus is that this results in unnecessary complexity in platform code and this can easily be done quickly on the JS side.