The code for the flexbox highlighter makes an incorrect assumption that all flex items are the flexbox container's children elements. See this line: https://searchfox.org/mozilla-central/rev/e06af9c36a73a27864302cd2f829e6200dee8541/devtools/server/actors/highlighters/flexbox.js#347 This can be incorrect for (at least) 3 cases: - Text nodes: if there is an unwrapped text node in between 2 flex items, then it becomes an anonymous flex item too. It can't be styled, but it should still be highlighted. Right now, it isn't. - Pseudo-elements: ::before and ::after pseudos of the flex container also become flex items. Right now the highlighter doesn't show them. - display:contents children: if a child of a flex container uses display:contents then it isn't rendered, but it's children are. So they should be highlighted too. This can be very easily fixed by using the node.getAsFlexContainer() chrome API which we already use anyway: node.getAsFlexContainer()[0].getItems() returns an array of items in the first line. I'll attach a test case that shows all of the cases that should be handled.

(In reply to Patrick Brosset <:pbro> from comment #0) > The code for the flexbox highlighter makes an incorrect assumption that all > flex items are the flexbox container's children elements. (This assumption is incorrect in the reverse direction, too - some children will be rendered but are not flex items, particularly the abspos/fixedpos children. I spun off bug 1435787 for that -- feel free to merge it into this issue if that's cleaner though.)

Would using .getItems() fix everything at once here? If so, I'm guessing we can just keep one of the 2 bugs.

Ah right, I forgot we added that API. I'm pretty sure that should be trustable to return the actual items (including/excluding [grand]children as-appropriate). So I agree that'd fix everything at once -- I'll dupe my abspos bug over to this one.

Comment on attachment 8956606 [details] Bug 1435749 - Get the correct flex items when rendering in the flexbox highlighter. https://reviewboard.mozilla.org/r/225536/#review231528 Very happy to see this! We'll need more changes throughout this file so justify-content is drawn correctly too, but it's a great start.

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5576eb6fee39 Get the correct flex items when rendering in the flexbox highlighter. r=pbro

Comment on attachment 8956739 [details] Bug 1435749 - Cut out justify-content areas using the correct flex items in the flexbox highlighter; https://reviewboard.mozilla.org/r/225704/#review231770 Remove the leave-open before landing. ::: devtools/server/actors/highlighters/flexbox.js:498 (Diff revision 1) > > let { bounds } = this.currentQuads.content[0]; > - let flexItems = this.currentNode.children; > let flexLines = this.currentNode.getAsFlexContainer().getLines(); > let computedStyle = getComputedStyle(this.currentNode); > - let direction = computedStyle.getPropertyValue("flex-direction"); > + let isColumn = computedStyle.getPropertyValue("flex-direction").startsWith("column"); I know we do this in other places so we should probably make this consistent by doing the same thing in every instance we need the flex direction. ::: devtools/server/actors/highlighters/flexbox.js:541 (Diff revision 1) > crossSize = Math.round(crossSize); > crossStart = Math.round(crossStart); > > - if (direction.startsWith("column") && > - crossStart <= left && > - left <= right && > + if (isColumn) { > + clearRect(this.ctx, crossStart, top, crossSize + crossStart, bottom, > + this.currentMatrix); Only one indent to make this consistent in this file. ::: devtools/server/actors/highlighters/flexbox.js:544 (Diff revision 1) > - if (direction.startsWith("column") && > - crossStart <= left && > - left <= right && > - right <= crossSize + crossStart) { > - // Remove the margin area for justify-content > - let marginTop = Math.round(parseFloat( > + if (isColumn) { > + clearRect(this.ctx, crossStart, top, crossSize + crossStart, bottom, > + this.currentMatrix); > + } else { > + clearRect(this.ctx, left, crossStart, right, crossSize + crossStart, > + this.currentMatrix); Same as above.

Pushed by pbrosset@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/71ecd11c2bf6 Cut out justify-content areas using the correct flex items in the flexbox highlighter; r=gl