1416231 - The main view may flicker while calculating the size of a subview

Status: RESOLVED DUPLICATE of bug 1392340

(Firefox :: Toolbars and Customization, defect, P1)

Description

[:Paolo Amadini]

This bug will be particularly visible after bug 1403469 is fixed.

This happens because the height of the off-screen view container currently influences the height of the visible views. This may have been a regression caused by one of the refactorings.

Comment 1

[:Paolo Amadini]

Attachment: Bug 1416231 - The main view may flicker while calculating the size of a subview.

This happened because the height of the off-screen view container influenced the height of the visible views, and is now fixed by setting the correct minimum and maximum heights independently.

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

Comment 2

[:Paolo Amadini]

Comment on attachment 8927341 [details]
Bug 1416231 - The main view may flicker while calculating the size of a subview.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198644/diff/1-2/

Comment 3

[:Paolo Amadini]

I rebased this patch on top of bug 1417042, as it can land independently from bug 1392340.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8927341 [details]
Bug 1416231 - The main view may flicker while calculating the size of a subview.

https://reviewboard.mozilla.org/r/198644/#review206554

I'm really sorry for the continued delays here. I just started looking at this, but I'm afraid I don't understand the patch. There are no STR, so I don't really understand what the issue is, how this patch fixes it, or why the individual bits of this patch are necessary/sufficient/helpful - even if they don't look harmful, to avoid ending up with a bunch of code that only the person who wrote it understand, I'd like to understand what is going on here better. Can you provide some more description than "This bug will be particularly visible after bug 1403469 is fixed." and the bug summary?

Comment 5

[:Paolo Amadini]

Folded this into bug 1392340, since all these changes are more related to the height calculation than the asynchronous description height workaround.

Comment 6

[:Paolo Amadini]

Attachment: Bug 1416231 - Add an asynchronous version of the description height workaround.

This prevents synchronous reflows when opening subviews. This also removes a superfluous invocation of the workaround while the panel is still hidden.

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

Comment 7

[Code Review Bot [:reviewbot]]

Comment on attachment 8954592 [details]
Bug 1416231 - Add an asynchronous version of the description height workaround.

https://reviewboard.mozilla.org/r/223688/#review229730


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/customizableui/PanelMultiView.jsm:1319
(Diff revision 1)
> -        continue;
> +          continue;
> -      }
> +        }
>  
> -      items.push({ element });
> +        items.push({ element });
> -    }
> +      }
> +    }

Error: Missing semicolon. [eslint: semi]

::: browser/components/customizableui/PanelMultiView.jsm:1338
(Diff revision 1)
>      // may contain wrapping text.
> +    let measureItems = () => {
> -    for (let item of items) {
> +      for (let item of items) {
> -      item.bounds = item.element.getBoundingClientRect();
> +        item.bounds = item.element.getBoundingClientRect();
> -    }
> +      }
> +    }

Error: Missing semicolon. [eslint: semi]