Julian Descottes [:jdescottes] Assignee
	Description • 8 years ago

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.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 1 • 8 years ago

Thanks for the report!

Honza

	Julian Descottes [:jdescottes] Assignee
	Comment 2 • 8 years ago

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.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 6 • 8 years ago
mozreview-review

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?

	Julian Descottes [:jdescottes] Assignee
	Comment 7 • 8 years ago

(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.

	Julian Descottes [:jdescottes] Assignee
	Comment 8 • 8 years ago

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

	Julian Descottes [:jdescottes] Assignee
	Comment 10 • 8 years ago

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

	Pulsebot
	Comment 11 • 8 years ago

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/771ccd7f6025
initialize splitter in horizontal mode if inspector starts in portrait mode;r=Honza

	Iris Hsiao [:ihsiao]
	Comment 12 • 8 years ago
bugherder

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

	Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact mboldan@mozilla.com
	Comment 13 • 8 years ago

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.