Closed Bug 1220388 Opened 4 years ago Closed 4 years ago

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

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(firefox45 affected, firefox50 verified)

VERIFIED FIXED
Firefox 50
Tracking Status
firefox45 --- affected
firefox50 --- verified

People

(Reporter: arni2033, Assigned: dalimil, Mentored)

References

Details

(Whiteboard: [good first bug][lang=js][btpp-backlog])

Attachments

(5 files, 2 obsolete files)

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
Whiteboard: [dupeme]
Has STR: --- → yes
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).
Mentor: pbrosset
Priority: -- → P3
Whiteboard: [dupeme] → [good first bug][lang=js][btpp-backlog]
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.
(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.
I would like to be assigned to this bug.
(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
Assignee: nobody → dalimilhajek
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.
Attached image 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...
I fixed the previous issues by using 'vw' as units. I set it to 40vw which seems to work well.
Attachment #8762388 - Flags: review?(pbrosset)
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.
Attachment #8762388 - Flags: review?(pbrosset) → review?(zer0)
See Also: → 1220389
:dalimil, I'm reviewing this patch, I'm testing some edge cases before. Thanks, and sorry for the waiting.
: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.
Attachment #8762388 - Flags: review?(zer0) → review-
Attached patch rev2 - flexbox solution (obsolete) — Splinter Review
Flexbox dark magic seems to have fixed everything.
Attachment #8762388 - Attachment is obsolete: true
Attachment #8766920 - Flags: review?(zer0)
Attachment #8766920 - Flags: review?(pbrosset)
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.
Attachment #8766920 - Flags: review?(pbrosset)
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.
Attachment #8766920 - Flags: review?(zer0) → review-
Attached patch rev3Splinter Review
I removed the redundant css properties.
Attachment #8766920 - Attachment is obsolete: true
Attachment #8768154 - Flags: review?(zer0)
Status: NEW → ASSIGNED
Comment on attachment 8768154 [details] [diff] [review]
rev3

LGTM, thanks for the review's comments addressed!
Attachment #8768154 - Flags: review?(zer0) → review+
Pushed by ntim.bugs@gmail.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/62265fa9b7d3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
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]
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
QA Whiteboard: [bugday-20160803]
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.