1303390 - Vertical layout is broken if sidebar width is > 425px

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Julian Descottes [:jdescottes]]

Attachment: splitter_vertical_layout.gif

Regression from Bug 1260552

STRs:
- open inspector in horizontal mode
- resize the splitter so that sidebar is 426px or more (alternatively you can set devtools.toolsidebar-width.inspector in about:config)
- resize the window so that the inspector switches to vertical layout

AR: layout doesn't change, only the "toggle sidebar" icon changes
ER: layout should change 

In inspector-panel.js::onPanelWindowResize, box.clientWidth always remains over 700.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1303390 - use width:100vw for inspector splitter container to avoid overflow;

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

Comment 2

[Julian Descottes [:jdescottes]]

Looks like this is another case of XUL/HTML layout issues. The fixes mentioned here also fix 1303402, which we will probably close as duplicate.

The current patch I submitted here is a workaround using width: 100vw on the inspector splitter container instead of width: 100% (this avoid the container's width to increase indefinitely when its content's width increases). There might very well be a better workaround. 

Also, I tried applying the patch from Bug 1262443 (migrating inspector.xul to html) => also solves the issue. 
But 1262443 depends on Bug 1294186 (dtd->properties migration) which will land only after merge day.

- land/uplift the workaround in this patch (or another)
- rework Bug 1262443 to keep using DTDs and land/uplift it before Bug 1294186 (AFAIK, that would still work, as long as the xhtml is loaded with chrome privileges)
- backout the splitter bug for now and land together with 126443 and 1294186

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8792181 [details]
Bug 1303390 - use width:100vw for inspector splitter container to avoid overflow;

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

Comment 4

[Brian Grinstead [:bgrins]]

Comment on attachment 8792181 [details]
Bug 1303390 - use width:100vw for inspector splitter container to avoid overflow;

https://reviewboard.mozilla.org/r/79386/#review78150

This is a good find, but the splitter patch has since been backed out.  We should probably fold this logic into the re-landing and keep this bug open for verification

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #4)
> Comment on attachment 8792181 [details]
> Bug 1303390 - use width:100vw for inspector splitter container to avoid
> overflow;
> 
> https://reviewboard.mozilla.org/r/79386/#review78150
> 
> This is a good find, but the splitter patch has since been backed out.  We
> should probably fold this logic into the re-landing and keep this bug open
> for verification

ni? Honza to make sure you see this comment and Comment 2.  There's a fix here for XUL/HTML layout to use vw instead of percentage width that we might want to take into bug 1260552.

Comment 6

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

Yes, I'll include the patch in bug 1260552. It could make tests a bit more stable.

@Julian, thanks for the report and patch!

And yes, let's keep this report for verification.

Honza

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/1bcfb236fe07
use width:100vw for inspector splitter container to avoid overflow;r=bgrins

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1bcfb236fe07

Comment 10

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified as fixed using latest Nightly 52.0a1 2016-09-22 across platforms.
Also checked that the duplicate bug 1303402 is no longer reproducible.