Closed Bug 1303390 Opened 3 years ago Closed 3 years ago

Vertical layout is broken if sidebar width is > 425px

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(firefox51 affected, firefox52 verified)

VERIFIED FIXED
Firefox 52
Iteration:
52.1 - Oct 3
Tracking Status
firefox51 --- affected
firefox52 --- verified

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

(Whiteboard: [devtools-html])

Attachments

(2 files)

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.
Blocks: 1260552
Flags: qe-verify+
QA Contact: petruta.rasa
Whiteboard: [devtools-html]
Priority: -- → P2
Blocks: 1303402
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
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Iteration: --- → 51.3 - Sep 19
Priority: P2 → P1
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
Attachment #8792181 - Flags: review?(bgrinstead)
(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.
Flags: needinfo?(odvarko)
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
Flags: needinfo?(odvarko)
Iteration: 51.3 - Sep 19 → 52.1 - Oct 3
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/1bcfb236fe07
use width:100vw for inspector splitter container to avoid overflow;r=bgrins
Duplicate of this bug: 1303402
https://hg.mozilla.org/mozilla-central/rev/1bcfb236fe07
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Verified as fixed using latest Nightly 52.0a1 2016-09-22 across platforms.
Also checked that the duplicate bug 1303402 is no longer reproducible.
Status: RESOLVED → VERIFIED
QA Whiteboard: [qe-dthtml]
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.