1444032 - Flex line highlighter isn't displayed correctly when justify-content is highlighted

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Patrick Brosset <:pbro>]

Attachment: flex-line.html

STR:
- open the attached test case
- open the inspector
- select the flex container and click in the rule-view to highlight the flexbox layout

--> Notice that the line that is normally displayed to indicate the existence of a flex line is partly hidden because of the justify-content areas.

Comment 1

[Patrick Brosset <:pbro>]

Attachment: missing-line.PNG

See the screenshot. I've highlighted in yellow where the line is missing.

Comment 2

[Patrick Brosset <:pbro>]

This can most probably be addressed simply by making reordering how we draw to the canvas. Indeed, we draw everything to just one canvas, and we clear some areas at times. So drawing in the right order should help.
The other option is to just never clear. Clear once before redrawing everything, and then just draw where needed.
The renderJustifyContent function can probably be changed to avoid clearing.

Comment 3

[Patrick Brosset <:pbro>]

Attachment: Bug 1444032 - Avoid clearing canvas when drawing justify-content areas;

So far, we are drawing the entire line area, and then cutting out the various
margin boxes from each item, in order to show where justify-content applies.
This is dangerous because that means clearing pixels in a canvas where we're
drawing tons of other stuff, and it's easy to accidentally clear unwanted
things.
Instead, now we just draw the justify-content where needed only.

Review commit: https://reviewboard.mozilla.org/r/226092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226092/

Comment 4

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

Comment on attachment 8957157 [details]
Bug 1444032 - Avoid clearing canvas when drawing justify-content areas;

https://reviewboard.mozilla.org/r/226092/#review232076

::: devtools/server/actors/highlighters/flexbox.js
(Diff revision 1)
> -    }
> -
> -    // Then, cut all the items out of this content area.
> -    for (let flexLine of flexLines) {
>        let flexItems = flexLine.getItems();
> -

Don't remove this new line. I prefer to keep the new lines to separate new if/for statement blocks.

::: devtools/server/actors/highlighters/flexbox.js
(Diff revision 1)
> -
>        for (let flexItem of flexItems) {
>          let { node } = flexItem;
>  
>          let quads = getAdjustedQuads(this.win, node, "margin");
> -

Don't remove this new line

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91b5d3f7409f
Avoid clearing canvas when drawing justify-content areas; r=gl

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/91b5d3f7409f