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: