1220388 - Element highlighter tooltip doesn't display element's size if it has long ID / firefox window is narrow

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[arni2033]

Attachment: testcase 1 - Element highlighter tooltip doesn't display element's size if it has long ID or firefox window is narrow.htm

STR:   (Win7_64, Nightly 45, 32bit, ID 20151029045227, new profile)
1. Open attached "testcase 1"
2. Open devtools -> Inspector, hover mouse over <div> element in markup-view

Result:
 Highlighter tooltip takes almost 100% of page [OK]. Element's size isn't visible in the tooltip

Expectations:
 Element's size should always be visible in highlight tooltip

Comment 1

[arni2033]

Attachment: screenshot - Element highlighter tooltip doesn't display element's size.png

Comment 2

[Patrick Brosset <:pbro>]

P3 because this is a very edge case, but I totally agree that the highlighter should handle it nicely and always show the information. We should have a max-width (which could depend on the window size) and use ellipsis for the various components (id, class).

Comment 3

[Daniele "Mte90" Scasciafratte]

On /devtools/server/actors/highlighters.css on ":-moz-native-anonymous .box-model-nodeinfobar-container" selector i see that have the max-width as 95%.
But is difficult to debug because with browser toolbox is not possible to maintain showed that tooltip and play with the css to found a solution.

Comment 4

[arni2033]

(In reply to Daniele "Mte90" Scasciafratte from comment #3)
> difficult to debug because with browser toolbox is not possible to maintain showed that tooltip
I can only suggest to read more my bugs, e.g. bug 1171491 comment 2 that would allow you to keep the tooltip visible, if that's the case.

Comment 5

[Dalimil Hajek [:dalimil]]

I would like to be assigned to this bug.

Comment 6

[Patrick Brosset <:pbro>]

(In reply to Daniele "Mte90" Scasciafratte from comment #3)
> On /devtools/server/actors/highlighters.css on ":-moz-native-anonymous
> .box-model-nodeinfobar-container" selector i see that have the max-width as
> 95%.
> But is difficult to debug because with browser toolbox is not possible to
> maintain showed that tooltip and play with the css to found a solution.
So, there is an easy way to keep the highlighter displayed:
- open gcli (the dev toolbar) with shift+F2
- type `highlight body --showinfobar`
- press enter

As long as you don't enter `highlight` or `unhighlight`, the highlighter will stay visible.

But, the problem is that the highlighter is injected in the page in a way that you can't inspect it with the browser toolbox. So even this isn't going to help much.

So the only way is to make changes to the CSS file, rebuild and check the changes.

(In reply to Dalimil Hajek [:dalimil] from comment #5)
> I would like to be assigned to this bug.
Great! I'm assigning the bug to you now.
Make sure you go through our contribution guide first: https://developer.mozilla.org/en-US/docs/Tools/Contributing

Comment 7

[Patrick Brosset <:pbro>]

The easiest way to fix this bug might be to extract the HTML and CSS of the tooltip into something like jsbin.com and investigate a fix there. At least you'd get direct feedback as you try various fixes.

In terms of code, the fix will most probably be in \devtools\server\actors\highlighters.css
If you search for 
.box-model-nodeinfobar-tagname
.box-model-nodeinfobar-id
.box-model-nodeinfobar-classes
.box-model-nodeinfobar-pseudo-classes
.box-model-nodeinfobar-dimensions
These are the classes used to display the various text elements inside the tooltip.
I'm guessing we'll need some max-width or flex layout, text-overflow, etc... that kind of stuff.

Comment 8

[Dalimil Hajek [:dalimil]]

Attachment: Screenshot-fix.png

I have a partially working solution... By adding the following css code to both .box-model-nodeinfobar-id and .box-model-nodeinfobar-classes:

+  display: inline-block; /* span does not allow max-width */
+  overflow: hidden;
+  vertical-align: top;
+  text-overflow: ellipsis;
+  max-width: 40%;

The additional whitespace at either side (seen in the screenshot) is caused by setting the max-width to 40% of the parent container (which doesn't have a fixed width). - I would like to set it to a percentage of the browser window width, but I don't know how to accomplish that...

Comment 9

[Dalimil Hajek [:dalimil]]

Attachment: rev1 - applies max-width (40% of browser width) for the 'id' and 'classes' fields

I fixed the previous issues by using 'vw' as units. I set it to 40vw which seems to work well.

Comment 10

[Patrick Brosset <:pbro>]

Sorry :dalimil for the delay. Last week we had a Mozilla-wide event that didn't leave any time for reviews, and this week I'm unfortunately very sick and won't be able to do the review either.
I'll re-assign it to Matteo (:zer0) who's a good reviewer for highlighter-related things.

Comment 11

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

:dalimil, I'm reviewing this patch, I'm testing some edge cases before. Thanks, and sorry for the waiting.

Comment 12

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

Attachment: screenshot - nodeinfobar on reduced width and different zoom factor

:dalimil, thanks for the patch! There are still a couple of situation we should cover. With your patch, we still having issue if we the window's size is small, or if we zoom out the page – see the screenshot attached.
You could give a try using the flexbox in the css, to ensure that the dimension box on the right is always displayed, no matter what.

Comment 13

[Dalimil Hajek [:dalimil]]

Attachment: rev2 - flexbox solution

Flexbox dark magic seems to have fixed everything.

Comment 14

[Patrick Brosset <:pbro>]

Comment on attachment 8766920 [details] [diff] [review]
rev2 - flexbox solution

Matteo started reviewing this patch, I'll let him finish. No need for 2 reviewers.

Comment 15

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

Comment on attachment 8766920 [details] [diff] [review]
rev2 - flexbox solution

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

Thanks for addressing my comments!
We can probably remove some css properties that I believe are not necessary; but please, check it and if I'm mistaken, add a comment explaining why they are.
After that, I think we can land it!

::: devtools/server/actors/highlighters.css
@@ +163,3 @@
>    padding-bottom: 1px;
> +  display: flex;
> +  flex-direction: row;

`row` should be the default value, so I think it's safe to omit.

@@ +163,4 @@
>    padding-bottom: 1px;
> +  display: flex;
> +  flex-direction: row;
> +  justify-content: center;

I'm not sure if we need it. The balloon has a `width` that is always "filled" by the content's item, so I don't think they have to be forcefully aligned. If you remove this property the end result should be the same.

@@ +167,5 @@
>  }
>  
>  :-moz-native-anonymous .box-model-nodeinfobar-tagname {
>    color: hsl(285,100%, 75%);
> +  flex-basis: content;

I don't think `flex-basis` is needed here. You should be achieve the same effect without specify this property. If it doesn't work as expected once you remove it, please add a comment explaining why is needed.

@@ +174,5 @@
>  :-moz-native-anonymous .box-model-nodeinfobar-id {
>    color: hsl(103, 46%, 54%);
> +  overflow: hidden;
> +  text-overflow: ellipsis;
> +  flex-basis: content;

As above.

@@ +182,5 @@
>  :-moz-native-anonymous .box-model-nodeinfobar-pseudo-classes {
>    color: hsl(200, 74%, 57%);
> +  overflow: hidden;
> +  text-overflow: ellipsis;
> +  flex-basis: content;

Ditto.

@@ +190,5 @@
>    color: hsl(210, 30%, 85%);
>    border-inline-start: 1px solid #5a6169;
>    margin-inline-start: 6px;
>    padding-inline-start: 6px;
> +  flex-basis: content;

Ditto.

Comment 16

[Dalimil Hajek [:dalimil]]

Attachment: rev3

I removed the redundant css properties.

Comment 17

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

Comment on attachment 8768154 [details] [diff] [review]
rev3

LGTM, thanks for the review's comments addressed!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/62265fa9b7d3
Element highlighter tooltip doesn't display element's size if it has long ID / firefox window is narrow. r=zer0

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/62265fa9b7d3

Comment 20

[Md. Rahimul Islam]

I have successfully reproduce this bug on firefox nightly 45.0a1 (2015-10-31)
with windows 7 (32 bit)
Mozilla/5.0 (Windows NT 6.1; rv:45.0) Gecko/20100101 Firefox/45.0

I found this fix on latest nightly 51.0a1 (2016-08-02)

Mozilla/5.0 (Windows NT 6.1; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID : 20160802030437

[bugday-20160803]

Comment 21

[Rezaul Huque Nayeem]

Reproduced this bug in firefox nightly 45.0a1 (2015-10-31) as comment 0  with ubuntu 16.04 (64 bit)

Verified this bug as fixed with latest firefox nightly 50.0a1 (Build ID: 20160801030227)
Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0