1304701 - Vertical layout is not initialized correctly in newly opened devtools

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Julian Descottes [:jdescottes]]

Attachment: splitter_always_initialized_vertically.gif

Regressions from Bug 1260552

STRs:

- open inspector
- resize the window so that inspector switches to vertical layout 
- close devtools
- open devtools

ER: Inspector should use vertical layout
AR: Inspector is in horizontal layout, but switches to vertical layout as soon as the window is resized.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1304701.patch

Thanks for the report!

Honza

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8793762 [details] [diff] [review]
bug1304701.patch

Review of attachment 8793762 [details] [diff] [review]:
-----------------------------------------------------------------

So I was working on a patch already and probably should have said so :) 

I added a test here as well.
Instead of doing an additional setState, I prefer to initialize the splitter with all the correct props from the beginning.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1304701 - initialize splitter in horizontal mode if inspector starts in portrait mode;

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

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8793767 [details]
Bug 1304701 - initialize splitter in horizontal mode if inspector starts in portrait mode;

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

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8793767 [details]
Bug 1304701 - initialize splitter in horizontal mode if inspector starts in portrait mode;

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

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8793767 [details]
Bug 1304701 - initialize splitter in horizontal mode if inspector starts in portrait mode;

https://reviewboard.mozilla.org/r/80432/#review79120

Nice, R+ and one question.

Honza

::: devtools/client/inspector/test/browser_inspector_portrait_mode.js:20
(Diff revision 3)
> +  let originalWidth = hostWindow.outerWidth;
> +  let originalHeight = hostWindow.outerHeight;
> +
> +  info("Resize toolbox window to force inspector to landscape mode");
> +  hostWindow.resizeTo(800, 500);
> +

How come you don't have to wait for class mutation in this case?

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> Comment on attachment 8793767 [details]
> Bug 1304701 - initialize splitter in horizontal mode if inspector starts in
> portrait mode;
> 
> https://reviewboard.mozilla.org/r/80432/#review79120
> 
> Nice, R+ and one question.
> 
> Honza
> 
> ::: devtools/client/inspector/test/browser_inspector_portrait_mode.js:20
> (Diff revision 3)
> > +  let originalWidth = hostWindow.outerWidth;
> > +  let originalHeight = hostWindow.outerHeight;
> > +
> > +  info("Resize toolbox window to force inspector to landscape mode");
> > +  hostWindow.resizeTo(800, 500);
> > +
> 
> How come you don't have to wait for class mutation in this case?

That's a good point. I don't need wait for a mutation because the inspector is already in landscape mode.
To be bullet-proof I should check the current layout, and resize only if the inspector in not in landscape mode. I'll update the test to do that.

Comment 8

[Julian Descottes [:jdescottes]]

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

Comment 9

[Julian Descottes [:jdescottes]]

Comment on attachment 8793767 [details]
Bug 1304701 - initialize splitter in horizontal mode if inspector starts in portrait mode;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80432/diff/3-4/

Comment 10

[Julian Descottes [:jdescottes]]

Updated try as per comment #7. Try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8d8825c302f

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/771ccd7f6025
initialize splitter in horizontal mode if inspector starts in portrait mode;r=Honza

Comment 12

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/771ccd7f6025

Comment 13

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this bug using Fx 52.0a1, build ID:20160922030437, on Windows 10 x64 and Mac OS X 10.10.5.
I can confirm this issue is fixed, I verified using Fx 52.0a1 build ID:20160927030200, on Windows 10 x64, Ubuntu 14.04 LTS and Mac OS X 10.12.