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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Rok Samsa]

Attachment: 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.

Comment 1

[Loic]

Any simple testcase to reproduce the issue in Inespector?

Comment 2

[Patrick Brosset <:pbro>]

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.

Comment 3

[Rok Samsa]

(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!

Comment 4

[Rok Samsa]

If I can look, and edit this problem, I would help, but for now I never fixed any bug :) I need a mentor.

Comment 5

[Patrick Brosset <:pbro>]

(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?

Comment 6

[Matteo Ferretti [:zer0] [:matteo]]

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

Comment 7

[Rok Samsa]

@Matteo I looked to this problem, and sorry, but rather you take a look. I'm not so much in JS.

Thank you! :)

Comment 8

[Rok Samsa]

What about this bug? This is still actual, and is hard to work with this bug! Can somebody look at this?

Comment 9

[Matteo Ferretti [:zer0] [:matteo]]

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?

Comment 10

[Gabriel Luong [: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.

Comment 11

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Bug 1255351 - box model highlighter: all the guidelines now are overlap the node;

This patch is fixing the issue mostly for 1dppx / 1dpi display.
For higher dpi a follow-up bug will be made once bug 1345917 and bug 1151421
are fixed; and it will be part of bug 1187970.

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

Comment 12

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

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 13

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

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

Comment 14

[Matteo Ferretti [:zer0] [:matteo]]

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."

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f152ca619a01
box model highlighter: all the guidelines now are overlap the node; r=gl

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f152ca619a01

Comment 17

[Madhuri]

Attachment: intersection points highlighted

[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

Comment 18

[Fahima Zulfath]

Attachment: moz.png

Comment 19

[Fahima Zulfath]

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.

Comment 20

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

Does this need to be reopen or is this expected because of the current limitations?

Comment 21

[Matteo Ferretti [:zer0] [:matteo]]

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!

Comment 22

[Madhuri]

Attachment: 1255351.png

Comment 23

[Madhuri]

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.