Closed Bug 1224932 Opened 9 years ago Closed 9 years ago

Box model is off-center

Categories

(DevTools :: Inspector, defect)

Product:
DevTools
Component:
Inspector
Type:
defect
Priority:
Not set
Severity:
trivial

Tracking

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

Status:
VERIFIED FIXED
Milestone:
Firefox 46
Tracking Flags:
Tracking Status
firefox43 --- unaffected
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: arni2033, Assigned: pbro)

References

Details

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

Attachments

(3 files)

2015.11.15 21-43-00.png
43.22 KB, image/png
Details
Bug_1224932_-_Center_the__body__element_in_the_lay.diff
997 bytes, patch
bgrins
: review+
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
Bug_1224932_-_2_-_Center_the__body__element_in_the.diff
1021 bytes, patch
pbro
: review+
ritu
: approval-mozilla-aurora+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached image 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.
Good catch on that regression. I did see it too last Friday and didn't file a bug then. Thanks for doing it.
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: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8699054 - Flags: review?(bgrinstead)
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: → 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)
Thanks Brian for confirming and filing this bug.
Keywords: leave-open
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.
https://hg.mozilla.org/mozilla-central/rev/e735b7607ae2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
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 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?
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+
status-b2g-v2.5: fixed → ---
Flags: needinfo?(cbook)
Has STR: --- → yes
[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
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: