Closed Bug 1255351 Opened 4 years ago Closed 3 years ago

Wrong border position of html element with padding or margin in Page Inspector (Developer tools)

Categories

(DevTools :: Inspector, defect, P3)

44 Branch
defect

Tracking

(firefox55 verified disabled)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- verified disabled

People

(Reporter: rok.samsa, Assigned: zer0)

References

(Blocks 1 open bug)

Details

(Whiteboard: [btpp-backlog])

Attachments

(5 files)

Attached image mozilla-bug.png
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

When I'm in developer mode, inspecting html elements on every web page, I have strange border positions.


Actual results:

One dashed border of element is not OK (usually top border). So borders are not in place.


Expected results:

All borders should be inside, and padding, or margin should be outside. So borders must be outside or inside.
Component: Untriaged → Developer Tools: Inspector
Mentor: nino.vranesic
Mentor: nino.vranesic
Any simple testcase to reproduce the issue in Inespector?
Flags: needinfo?(rok.samsa)
STR:
- open the inspector on this page
- click on the "select node" icon in the toolbar
- move your mouse over elements in the page

If you look closely (or take a screenshot and then zoom in), you can see that the highlighter guides (the vertical and horizontal blue dashed lines) are always displayed starting from the region they wrap.

So if this is the element:

   +-------------------+
   |                   |
   |                   |
   |                   |
   |                   |
   +-------------------+

Then the guides would be displayed as follows:

   +-------------------+
-- -- -- -- -- -- -- -- -- --
   |                   |
   |                   |
   |                   |
   |                   |
   +-------------------+
-- -- -- -- -- -- -- -- -- --

(Only representing the horizontal guides here, and the distance is exaggerated with the ascii art here on purpose).

I think the reporter is asking for it to be this instead:

   +-------------------+
-- -- -- -- -- -- -- -- -- --
   |                   |
   |                   |
   |                   |
   |                   |
-- -- -- -- -- -- -- -- -- --
   +-------------------+

Which makes sense.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(rok.samsa)
Priority: -- → P3
Whiteboard: [btpp-backlog]
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #2)
> STR:
> - open the inspector on this page
> - click on the "select node" icon in the toolbar
> - move your mouse over elements in the page
> 
> If you look closely (or take a screenshot and then zoom in), you can see
> that the highlighter guides (the vertical and horizontal blue dashed lines)
> are always displayed starting from the region they wrap.
> 
> So if this is the element:
> 
>    +-------------------+
>    |                   |
>    |                   |
>    |                   |
>    |                   |
>    +-------------------+
> 
> Then the guides would be displayed as follows:
> 
>    +-------------------+
> -- -- -- -- -- -- -- -- -- --
>    |                   |
>    |                   |
>    |                   |
>    |                   |
>    +-------------------+
> -- -- -- -- -- -- -- -- -- --
> 
> (Only representing the horizontal guides here, and the distance is
> exaggerated with the ascii art here on purpose).
> 
> I think the reporter is asking for it to be this instead:
> 
>    +-------------------+
> -- -- -- -- -- -- -- -- -- --
>    |                   |
>    |                   |
>    |                   |
>    |                   |
> -- -- -- -- -- -- -- -- -- --
>    +-------------------+
> 
> Which makes sense.

Exactly! Thank you Patrick, I just got ready, to write detailed info about this problem, and I saw that you already do that insted of me. Thank you!
If I can look, and edit this problem, I would help, but for now I never fixed any bug :) I need a mentor.
(In reply to Rok Samsa from comment #4)
> If I can look, and edit this problem, I would help, but for now I never
> fixed any bug :) I need a mentor.
The first thing to do is to go through our hacking guide: https://wiki.mozilla.org/DevTools/Hacking
Feel free to ask questions on IRC #devtools: https://wiki.mozilla.org/DevTools/GetInvolved#Communication
As for a mentor, I think Matteo (:zer0) would be best for this bug.

@Matteo: can you give some pointers to Rok on how to solve this bug?
Flags: needinfo?(zer0)
I'm glad to help!

@Rok, I would start to look at the highlighter responsible for that, the box model highlighter [1]. Specifically, I would check the `_showGuides`[2] function.

And feel free to ask anything; you can "needinfo" me, send me an email or find on IRC (my nickname is zer0). 

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/highlighters/box-model.js
[2] https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/highlighters/box-model.js#575
Flags: needinfo?(zer0)
@Matteo I looked to this problem, and sorry, but rather you take a look. I'm not so much in JS.

Thank you! :)
What about this bug? This is still actual, and is hard to work with this bug! Can somebody look at this?
Assignee: nobody → zer0
This bug can't really be fixed properly until bug 1345917 and bug 1151421 are fixed.

We could fix just for 1dppx (with no zoom / retina display), and then create a follow-up bug for when we can handle the higher dppx, but I'm not sure if it's worth.

Gabriel, what do you think? Rok?
Flags: needinfo?(rok.samsa)
Flags: needinfo?(gl)
It might be worthwhile to fix this in 1dppx, but I would consider the amount of time required given our current priorities. If this is a quick fix, go for it, otherwise, I would wait for the proper fix.
Flags: needinfo?(gl)
Flags: needinfo?(rok.samsa)
Comment on attachment 8845700 [details]
Bug 1255351 - box model highlighter: all the guidelines now are overlap the node;

https://reviewboard.mozilla.org/r/118846/#review121262

::: devtools/client/shared/test/test-actor.js:1063
(Diff revision 1)
>        return null;
>      }
>  
>      return {
>        p1: {x: lGuide.x1, y: tGuide.y1},
> -      p2: {x: rGuide.x1, y: tGuide. y1},
> +      p2: {x: +rGuide.x1 + 1, y: tGuide.y1},

I am not too familiar with the math here, can you explain what is happening to these p# values?

::: devtools/server/actors/highlighters/box-model.js:604
(Diff revision 1)
>          }
>        }
>      }
>  
>      // Move guide into place or hide it if no valid co-ordinate was found.
> -    this._updateGuide("top", toShowY[0]);
> +    this._updateGuide("top", Math.round(toShowY[0]));

Can you explain why we round the value and -1 for "right" and "bottom"? I figure we would be modifying the top line.
Comment on attachment 8845700 [details]
Bug 1255351 - box model highlighter: all the guidelines now are overlap the node;

https://reviewboard.mozilla.org/r/118846/#review121526
Attachment #8845700 - Flags: review?(gl) → review+
Comment on attachment 8845700 [details]
Bug 1255351 - box model highlighter: all the guidelines now are overlap the node;

https://reviewboard.mozilla.org/r/118846/#review121262

> I am not too familiar with the math here, can you explain what is happening to these p# values?

We cleared that on face to face (as we said the values are string).

> Can you explain why we round the value and -1 for "right" and "bottom"? I figure we would be modifying the top line.

We cleared that over IRC:
"To be clear, if you have a <div> positioned absolutely, top:10px, left: 10px, and with size: 100x100px; now the top guideline is positioned at 10px, the left at 10px; and the bottom and right, at 110px.
Therefore, the top and left are "inside" the div, where the right and bottom are "outside" the div."
Pushed by mferretti@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f152ca619a01
box model highlighter: all the guidelines now are overlap the node; r=gl
https://hg.mozilla.org/mozilla-central/rev/f152ca619a01
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
[bugday-20170329]

status-ff55 : NOT FIXED.

Managed to reproduce the issue on Firefox Nightly(affected build ID:20160210153822), under Windows 10 X 64.

The issue is still reproducible on Firefox latest Nightly [BuildID : 20170328095415 , 55.0a1(2017-03-28)(32 bit)]. 
Tests were performed under Windows 10 X 64.

NOTE - Please refer the screenshot attached
Attached image moz.png
Comment on attachment 8854970 [details]
moz.png

[BugDay-20170405]

Wrong border position of html element with padding or margin in Page Inspector (Developer tools) is still reproducible on Nightly 55.0a1(2017-04-05) 64 bit
OS: windows 10.
Does this need to be reopen or is this expected because of the current limitations?
Flags: needinfo?(zer0)
If this is happening on 1ddpx, no zoom, no scrolling, just plain inspection, then we definitely need to reopen the bug.

In those condition, if it's happening only on Windows it could be also that there is a platform difference we're not aware of.

Madhuri, coul you let us know specifically the conditions where the issue was reproduced (display density, zoom level, if the page was previously scrolled); and the give to us the page's URL that was used for such test, so we can try to reproduce it.

Thanks!
Flags: needinfo?(zer0) → needinfo?(madhuri.mittal99)
Attached image 1255351.png
Flags: needinfo?(madhuri.mittal99)
Today I just saw on this page only (refer screenshot attached above) on Win 10 X 64 with build ID : 20170912013600 (FF-Nightly 57.0a1).

No scrolling or zooming , just plain inspection. Please let me know if I can help more.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.