1521604 - Add parentFlexElement property to HTML elements

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P2)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Martin pointed out that since landing the combined highlighter (flexbox + box model) the highlighter itself has become laggy.

This is because for each node that is selected we need to walk up the DOM tree to see if a node is inside a flexbox container.

We should add a chromeOnly property to flex items that will give us their container.

This should be as performant as possible.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@Brad Is this something you could do?

Comment 2

[Brad Werth [:bradwerth]]

This should be straightforward. (Editing) Hmmm... less so. If we need to retrieve this for all HTML elements, then we don't have a good existing hook for this. The way we get "extra" information for grid and flex containers is to do a reflow that generates that extra data when needed. For grid and flex, we only do this when the relevant element is selected in by the inspector. In this case, we would be doing a reflow of the whole document as soon as we open the highlighter, which could be a bad user experience. I'll thikn on this and see if I can come up with a better solution.

Comment 3

[Brad Werth [:bradwerth]]

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

This is because for each node that is selected we need to walk up the DOM tree to see if a node is inside a flexbox container.

Mike, would you please link to the JS implementation of the walk up the DOM tree so I can replicate the algorithm correctly?

Comment 4

[Brad Werth [:bradwerth]]

Attachment: Bug 1521604 Part 2: Create a Node chrome-only parentFlexElement property, for use by devtools.

Comment 5

[Brad Werth [:bradwerth]]

The attachment 9038403 [details] provides a method of doing a very short DOM walk in the platform code to return the parent flex element, if any. If this works and is performant enough, then there's no need to do a decorating reflow of the document to cache pointers to the flex parents.

Comment 6

[Brad Werth [:bradwerth]]

Attachment: Bug 1521604 Part 3: Add a test of parentFlexElement.

Depends on D17308

Comment 7

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0900282c8c0b7fe7109643360806c53fb3bf594

Comment 8

[Brad Werth [:bradwerth]]

Attachment: Bug 1521604 Part 1: Hoist the flushing version of GetPrimaryFrame from Element to nsIContent.

Comment 9

[Brad Werth [:bradwerth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49ad7bb661526bd7250503ad0e0d945dc9f4a73d

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d3b8198f399
Part 1: Hoist the flushing version of GetPrimaryFrame from Element to nsIContent. r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/e23801bb3b78
Part 2: Create a Node chrome-only parentFlexElement property, for use by devtools. r=dholbert,bzbarsky
https://hg.mozilla.org/integration/autoland/rev/d39849743a9c
Part 3: Add a test of parentFlexElement. r=dholbert

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6d3b8198f399
https://hg.mozilla.org/mozilla-central/rev/e23801bb3b78
https://hg.mozilla.org/mozilla-central/rev/d39849743a9c