1348005 - Grid outline needs to update on reflow

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

Gabriel, could you elaborate a bit this bug?

Comment 2

[Julian Descottes [:jdescottes]]

Can we have a scenario or test page that highlights the issue here?

Comment 3

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

STR.
1. Enable grid outline - search for gridoutline in about:config
2. Go to http://labs.jensimmons.com/2016/examples/grid-content-1.html
3. Observe grid outline when resizing the page

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: grid_outline_resize.gif

I still don't get what is wrong here :/

The layout seems to update when resizing the window (you can check the attached GIF). What needs to be changed here?

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1348005 - update grid panel on reflows;

Extracted the reflow tracking logic from the box-model to a dedicated util in
inspector/shared/reflow-tracker.js to use it in both box-model and grid-inspector.

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

Comment 6

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

Comment on attachment 8852618 [details]
Bug 1348005 - update grid panel on reflows;

https://reviewboard.mozilla.org/r/124808/#review127350

::: devtools/client/inspector/boxmodel/box-model.js:49
(Diff revision 1)
>    this.onShowBoxModelEditor = this.onShowBoxModelEditor.bind(this);
>    this.onShowBoxModelHighlighter = this.onShowBoxModelHighlighter.bind(this);
>    this.onSidebarSelect = this.onSidebarSelect.bind(this);
>    this.onToggleGeometryEditor = this.onToggleGeometryEditor.bind(this);
>  
> +  this.inspector.reflowTracker.on("reflow", this.updateBoxModel);

This should only be on when the sidebar is selected.

::: devtools/client/inspector/grids/grid-inspector.js:89
(Diff revision 1)
>      );
>  
>      this.highlighters.on("grid-highlighter-hidden", this.onHighlighterChange);
>      this.highlighters.on("grid-highlighter-shown", this.onHighlighterChange);
>      this.inspector.on("markupmutation", this.onMarkupMutation);
> +    this.inspector.reflowTracker.on("reflow", this.onReflow);

This should only be on when the sidebar is selecte.d

::: devtools/client/inspector/grids/grid-inspector.js:105
(Diff revision 1)
>      this.highlighters.off("grid-highlighter-hidden", this.onHighlighterChange);
>      this.highlighters.off("grid-highlighter-shown", this.onHighlighterChange);
>      this.inspector.off("markupmutation", this.onMarkupMutation);
> +    this.inspector.reflowTracker.off("reflow", this.onReflow);
>      this.inspector.sidebar.off("select", this.onSidebarSelect);
> +

I dont think we need this new line

::: devtools/client/inspector/inspector.js:99
(Diff revision 1)
>    this.panelDoc = window.document;
>    this.panelWin = window;
>    this.panelWin.inspector = this;
>  
>    this.highlighters = new HighlightersOverlay(this);
> +  this.reflowTracker = new ReflowTracker(this._target);

We need to make sure we destroy this (this.reflowTracker = null) and have a destroy method inside ReflowTracker to prevent leaks.

::: devtools/client/inspector/shared/reflow-tracker.js:58
(Diff revision 1)
> +    if (this.listeners.has(listener)) {
> +      return;
> +    }
> +
> +    // No listener interested in reflows yet, start tracking.
> +    if (this.count === 0) {

This can simply be if (this.count === 0 && this.reflowFront)

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8852618 [details]
Bug 1348005 - update grid panel on reflows;

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

Comment 8

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

Comment on attachment 8852618 [details]
Bug 1348005 - update grid panel on reflows;

https://reviewboard.mozilla.org/r/124808/#review127684

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d7947c28ab4f
update grid panel on reflows;r=gl

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d7947c28ab4f