Box model is off-center

VERIFIED FIXED in Firefox 44

Status

()

Firefox
Developer Tools: Inspector
--
trivial
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: arni2033, Assigned: pbro)

Tracking

({regression})

Trunk
Firefox 46
regression
Points:
---

Firefox Tracking Flags

(firefox43 unaffected, firefox44 fixed, firefox45 fixed, firefox46 fixed)

Details

(Whiteboard: [good first bug][lang=css])

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Created attachment 8687690 [details]
2015.11.15 21-43-00.png

>>> My Info:   Win7_64, Nightly 45, 32bit, ID 20151113030248, new profile <<<
STR:
1. Open devtools -> Inspector -> Box model on this page, resize the inspector sidebar to at least 500px

Result:         Box model is aligned at the left
Expectations:   Box model should be placed at the center

Regression window:
>   https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a0e7e27c0872696dd10708b96198f7fe19941c52&tochange=e2ee8f447aa139ac476058c1f8d657bcc0ade30f

Note:
<body> had "margin: 0px auto;" before bug 1215418, and not it's overriden by "margin: 0px;"

[OOT]
Looks like I'm the only person who used Box-model in the past month. Btw I noticed that on October ~29th when updated my Dev.Edition 43 -> 44.
(Assignee)

Comment 1

2 years ago
Good catch on that regression. I did see it too last Friday and didn't file a bug then. Thanks for doing it.
(Assignee)

Comment 2

2 years ago
Bug 1215418 did introduce body{margin:0} in light-theme.css and dark-theme.css

What I can't figure out is why these stylesheets override styles in panel's CSS files.

Like in the layout-view, I would expect the theme CSS (that contains body{margin:0}) to be loaded first and then the panel's CSS (layoutview.css, which contains body{margin:0 auto;}) to be loaded next and therefore override the body margin.

Since this is a very small bug to fix and has been present in dev-edition 44 for a while, I'm tempted to just patch this, but NI? Brian who might know if we can change the way stylesheets are loaded.
Flags: needinfo?(bgrinstead)
(Assignee)

Comment 3

2 years ago
Created attachment 8699054 [details] [diff] [review]
Bug_1224932_-_Center_the__body__element_in_the_lay.diff
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8699054 - Flags: review?(bgrinstead)
(Assignee)

Comment 4

2 years ago
Actually we're past the merge date, so if we want to fix this in 44, we need to request uplift to beta.
See Also: → bug 1233127
Comment on attachment 8699054 [details] [diff] [review]
Bug_1224932_-_Center_the__body__element_in_the_lay.diff

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

Works for me.  I filed Bug 1233127 to fix this properly
Attachment #8699054 - Flags: review?(bgrinstead) → review+
Let's just patch this for now and fix the root issue in  Bug 1233127
Flags: needinfo?(bgrinstead)
(Assignee)

Comment 7

2 years ago
Thanks Brian for confirming and filing this bug.

Comment 8

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/e735b7607ae2
(Assignee)

Updated

2 years ago
Keywords: leave-open
(Assignee)

Comment 9

2 years ago
Ah, turns out the patch is wrong. Yes it did result in centering the layout-view, but only because the selector I chose was wrong and therefore the whole rule didn't apply, including the max-width:400px. Without this property, the layout-view appears centered because it's essentially full-width.

To properly fix this, I'll attach a new patch, trivial CSS change that I'll review myself and land quickly.
(Assignee)

Comment 10

2 years ago
Created attachment 8699415 [details] [diff] [review]
Bug_1224932_-_2_-_Center_the__body__element_in_the.diff
Attachment #8699415 - Flags: review+
(Assignee)

Comment 11

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/1e4c936c142d
Keywords: leave-open

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e735b7607ae2
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(Assignee)

Comment 13

2 years ago
Can attachment 8699415 [details] [diff] [review] also be landed on m-c?
Flags: needinfo?(cbook)
Reproduced this bug with Firefox Nightly 45.0a1 (2015-11-15);(Build ID: 20151115030440) on Linux, 64 Bit

This Bug is now verified as fixed on Latest Firefox Nightly 46.0a1 (2015-12-17)

Build ID: 20151217072309
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
QA Whiteboard: [bugday-20151216]

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1e4c936c142d
(Assignee)

Comment 16

2 years ago
Comment on attachment 8699054 [details] [diff] [review]
Bug_1224932_-_Center_the__body__element_in_the_lay.diff

Approval Request Comment

This is the approval request for part 1.

[Feature/regressing bug #]: 1215418
[User impact if declined]: The content of the "box model" inspector tab won't be properly centered. The tab is still usable, just not as nice.
[Describe test coverage new/current, TreeHerder]: No test coverage for this CSS only layout issue. This has landed on m-c though.
[Risks and why]: CSS-only centering change, really no risk at all. Even if a mistake was made, the layout would just be mis-aligned, but still usable.
[String/UUID change made/needed]: None
Attachment #8699054 - Flags: approval-mozilla-beta?
Attachment #8699054 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 17

2 years ago
Comment on attachment 8699415 [details] [diff] [review]
Bug_1224932_-_2_-_Center_the__body__element_in_the.diff

Approval Request Comment

This is the approval request for part 2.

[Feature/regressing bug #]: 1215418
[User impact if declined]: The content of the "box model" inspector tab won't be properly centered. The tab is still usable, just not as nice.
[Describe test coverage new/current, TreeHerder]: No test coverage for this CSS only layout issue. This has landed on m-c though.
[Risks and why]: CSS-only centering change, really no risk at all. Even if a mistake was made, the layout would just be mis-aligned, but still usable.
[String/UUID change made/needed]: None
Attachment #8699415 - Flags: approval-mozilla-beta?
Attachment #8699415 - Flags: approval-mozilla-aurora?
Verified fixed based on comment 14. Thanks Nazir!
Status: RESOLVED → VERIFIED
Comment on attachment 8699054 [details] [diff] [review]
Bug_1224932_-_Center_the__body__element_in_the_lay.diff

The fix was verified on Nightly. Beta44+, Aurora45+
Attachment #8699054 - Flags: approval-mozilla-beta?
Attachment #8699054 - Flags: approval-mozilla-beta+
Attachment #8699054 - Flags: approval-mozilla-aurora?
Attachment #8699054 - Flags: approval-mozilla-aurora+
Comment on attachment 8699415 [details] [diff] [review]
Bug_1224932_-_2_-_Center_the__body__element_in_the.diff

Beta44+, Aurora45+
Attachment #8699415 - Flags: approval-mozilla-beta?
Attachment #8699415 - Flags: approval-mozilla-beta+
Attachment #8699415 - Flags: approval-mozilla-aurora?
Attachment #8699415 - Flags: approval-mozilla-aurora+

Comment 21

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/befd77fc145b
https://hg.mozilla.org/releases/mozilla-aurora/rev/11273dc5355c
status-firefox45: affected → fixed

Comment 22

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/dda18c6947b2
https://hg.mozilla.org/releases/mozilla-beta/rev/a011fb29cdc0
status-firefox44: affected → fixed

Comment 23

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/dda18c6947b2
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a011fb29cdc0
status-b2g-v2.5: --- → fixed

Updated

2 years ago
status-b2g-v2.5: fixed → ---
Flags: needinfo?(cbook)
(Reporter)

Updated

2 years ago
Has STR: --- → yes

Comment 24

2 years ago
[bugday-20160323]

Status: RESOLVED,FIXED -> VERIFIED

Comments:
Test Successful.

Component:
Name 			Firefox
Version 		46.0b9
Build ID 		20160322075646
Update Channel 	        beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS                      Windows 7 SP1 x86_64

Expected Results: 
In inspector, Box model is at center.

Actual Results: 
As expected
You need to log in before you can comment on or make changes to this bug.