1351685 - Remove the box model from the computed view

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Title says most of it. 

We currently have duplicated box-model widget in both the layout & computed views. It's confusing, users can't easily tell where they are since both panels look the same etc...

Does anyone share this concern? Do we plan to take a decision here before shipping the layout panel? 

I can see a few options:
- simply remove the box-model widget from the computed view
- hide the box-model widget from the computed view if and only if the layout panel is enabled

Comment 1

[Patrick Brosset <:pbro>]

As I recall, the original idea was to have a simple box-model widget in the computed-view, something you can quickly look at to find computed values for the margin, border, padding, content. And the layout panel would contain a much more interesting view of it, with more properties, more information, but which would occupy more room too.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: layout_vs_computed_panel.gif

My main issue is that when the devtools are docked to the bottom, both panels look identical at first glance.

Here's a GIF to illustrate this. But if I'm the only one finding this confusing, feel free to close!

Comment 3

[Soledad Penades [:sole] [:spenades]]

No, you're not the only one who's confused by the similarity between the two panels :-)

Comment 4

[Tim Nguyen :ntim]

How about making the box model section in the computed view a link to the layout view ?

Something like this:

Rules | [Computed] | Layout | Animations
---------------------------------------
[ Filter Styles      ][ ] Browser styles
---------------------------------------
Box model                           >
---------------------------------------
...

Or an infobar like in the Layout panel:

(i) The box-model view has moved to the [layout panel]      (x)

Comment 5

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

It was decided for the 3 pane inspector that we would remove the box model from the computed view. https://docs.google.com/document/d/1FDhZp-Qd9u38l14t8eU-hPG54Na7GqbBDFQWraNIyws/edit#

Comment 6

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

Attachment: Bug 1351685 - Remove the box model from the computed view.

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

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 8956687 [details]
Bug 1351685 - Remove the box model from the computed view.

https://reviewboard.mozilla.org/r/225640/#review231524

Looks good.
Please land this only during the 61 cycle, so that it ships with 3-pane.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57ba784a1cd9
Remove the box model from the computed view. r=pbro

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c1886396b55
Backed out changeset 57ba784a1cd9 for ESlint failure on browser_boxmodel_computed-accordion-state.js

Comment 10

[Cosmin Sabou [:CosminS]]

Backed out changeset for ESlint failure on browser_boxmodel_computed-accordion-state.js 

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=57ba784a1cd94404c746db1ca381cfc2661506be&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=167440897&filter-searchStr=bf1813c891520f2ba58ac0e5c348b469a6fe7762

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=167440897&repo=mozilla-inbound&lineNumber=253

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/6c1886396b55cf2ba68e864d11e5aeffe92364f0

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/139930cd68e2
Remove the box model from the computed view. r=pbro

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/04cb6477bb38
Backed out changeset 139930cd68e2 for xpcshell failures on test_ext_webRequest_responseBody.js CLOSED TREE

Comment 13

[Cristian Brindusan [:cbrindusan]]

Backed out changeset for xpcshell failures on test_ext_webRequest_responseBody.js 

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=139930cd68e2d8f138837b282b8101e550fc5f41&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=167520053&repo=mozilla-inbound&lineNumber=1545

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/04cb6477bb38b14e4b8831eb7f599e5878e7a4bb

Comment 14

[Cristian Brindusan [:cbrindusan]]

Sorry about the previous message, the backout was for devtools failures.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bae5dd6b9a2a
Remove the box model from the computed view. r=pbro

Comment 16

[Narcis Beleuzu [:NarcisB]]

Backed out for devtools failres on browser_boxmodel_editablemodel.js

Log: https://treeherder.mozilla.org/logviewer.html#?job_id=167609905&repo=mozilla-inbound&lineNumber=1800

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/961f8637533b
Backed out changeset bae5dd6b9a2a for devtools failres on browser_boxmodel_editablemodel.js

Comment 18

[Julian Descottes [:jdescottes]]

For information the test fails because the toolbox is too small and the previous focus the element below the boxmodel. The box model top fields become scrolled out and the click fails.

Either:
- increase the toolbox height: 
  yield pushPref("devtools.toolbox.footer.height", 500);
- ensure the box model container is fully visible
  let container = boxmodel.document.querySelector(".boxmodel-container");
  container.scrollIntoView();

Comment 19

[Julian Descottes [:jdescottes]]

Forgot to mention: the test that fails is testRefocusingOnClick(), not sure exactly which previous test is moving the focus and scrolling out the container, but if you decide to scrollIntoView the boxmodel container, it would probably make sense to do it in each test method.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d9cd8bb925d
Remove the box model from the computed view. r=pbro

Comment 21

[Arthur Iakab [arthur_iakab]]

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

Comment 22

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 55.0a1 (2017-03-29) on Windows 10, 64 Bit!

This bug's fix is verified with latest Nightly!

Build ID   : 20180419224145
User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0