Web Inspector: CPU Usage Timeline - Give long thread names a tooltip if they could be ellipsized

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

CPU Usage Timeline - Give long thread names a tooltip if they could be ellipsized

Example: The Worker on maps.google.com

[Joseph Pecoraro]

Created attachment 363602 [details]
[PATCH] Proposed Fix

[Nikita Vasilyev]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=363602&action=review

> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:42
> +            if (displayName.length >= 20)
> +                detailsNameElement.title = displayName;

I think this is fine because `.cpu-usage-view > .details` is fixed at 150px. I think it deserves a comment on how you came up with the number 20.

If the width wasn't fixed, you could check if the text is overflowing by `element.scrollWidth > element.offsetWidth`.

[EWS Watchlist]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

Attachment 363602 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11374494

New failing tests:
editing/selection/thai-word-at-document-end.html

[EWS Watchlist]

Created attachment 363622 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Devin Rousso]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=363602&action=review

> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:41
> +            if (displayName.length >= 20)

I don't like the idea of only adding a tooltip if the name is long enough.  I think we should be consistent and always add it.

[Joseph Pecoraro]

(In reply to Devin Rousso from comment #5)
> Comment on attachment 363602 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363602&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:41
> > +            if (displayName.length >= 20)
> 
> I don't like the idea of only adding a tooltip if the name is long enough. 
> I think we should be consistent and always add it.

Adding tooltips all over is a bad idea. Then the user can't rest their mouse anywhere without tooltips showing up all over the place, which is a poor experience.

[Matt Baker]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=363602&action=review

>> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:42
>> +                detailsNameElement.title = displayName;
> 
> I think this is fine because `.cpu-usage-view > .details` is fixed at 150px. I think it deserves a comment on how you came up with the number 20.
> 
> If the width wasn't fixed, you could check if the text is overflowing by `element.scrollWidth > element.offsetWidth`.

I like Nikita's suggestion. It also eliminates issues due to different character widths.

[Joseph Pecoraro]

(In reply to Matt Baker from comment #7)
> Comment on attachment 363602 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363602&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:42
> >> +                detailsNameElement.title = displayName;
> > 
> > I think this is fine because `.cpu-usage-view > .details` is fixed at 150px. I think it deserves a comment on how you came up with the number 20.
> > 
> > If the width wasn't fixed, you could check if the text is overflowing by `element.scrollWidth > element.offsetWidth`.
> 
> I like Nikita's suggestion. It also eliminates issues due to different
> character widths.

I believe this would cause multiple forced layouts (depending on the number of sections) in the initial layout, which seems unnecessary.

The number 20 here is a conservative estimate to avoid having too many tooltips on sections that would obviously not have clipped text.

[Devin Rousso]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=363602&action=review

>>> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:41
>>> +            if (displayName.length >= 20)
>> 
>> I don't like the idea of only adding a tooltip if the name is long enough.  I think we should be consistent and always add it.
> 
> Adding tooltips all over is a bad idea. Then the user can't rest their mouse anywhere without tooltips showing up all over the place, which is a poor experience.

This isn't "all over".  This is in a place where it's not uncommon for data to be clipped.  I would find it more weird as a user to hover something and have it show a tooltip, and then hover something right next to it and not have it show a tooltip.  I'd expect consistency.

[Joseph Pecoraro]

(In reply to Devin Rousso from comment #9)
> Comment on attachment 363602 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363602&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:41
> >>> +            if (displayName.length >= 20)
> >> 
> >> I don't like the idea of only adding a tooltip if the name is long enough.  I think we should be consistent and always add it.
> > 
> > Adding tooltips all over is a bad idea. Then the user can't rest their mouse anywhere without tooltips showing up all over the place, which is a poor experience.
> 
> This isn't "all over".  This is in a place where it's not uncommon for data
> to be clipped.  I would find it more weird as a user to hover something and
> have it show a tooltip, and then hover something right next to it and not
> have it show a tooltip.  I'd expect consistency.

If I get a tooltip on text that matches the exact text I'd consider that a bug. Ideally it only happens on ellipsized text.

We do consistently always put tooltips on non-text items, like navigation items, tree elements, and links. But I don't think we do that for just text / titles across the system.

Does anyone have an example on the system of repeating a tooltip for non-ellipsized text?

[Devin Rousso]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=363602&action=review

>>>>> Source/WebInspectorUI/UserInterface/Views/CPUUsageView.js:41
>>>>> +            if (displayName.length >= 20)
>>>> 
>>>> I don't like the idea of only adding a tooltip if the name is long enough.  I think we should be consistent and always add it.
>>> 
>>> Adding tooltips all over is a bad idea. Then the user can't rest their mouse anywhere without tooltips showing up all over the place, which is a poor experience.
>> 
>> This isn't "all over".  This is in a place where it's not uncommon for data to be clipped.  I would find it more weird as a user to hover something and have it show a tooltip, and then hover something right next to it and not have it show a tooltip.  I'd expect consistency.
> 
> If I get a tooltip on text that matches the exact text I'd consider that a bug. Ideally it only happens on ellipsized text.
> 
> We do consistently always put tooltips on non-text items, like navigation items, tree elements, and links. But I don't think we do that for just text / titles across the system.
> 
> Does anyone have an example on the system of repeating a tooltip for non-ellipsized text?

I don't have an example of the system showing a consistent tooltip, but the system also only shows a tooltip if text is overflowing.  We don't perform that check (we could though by using `sizeDidChange` and some `offsetWidth` comparisons).  We do, however, always show tooltips for all values in a `WI.DataGrid` regardless of whether it is overflowing, because of the fact that we don't want to have to do that logic.  I think we should follow that pattern, because it's worse to not have a tooltip for overflowing text than it is to have a tooltip for non-overflowing text.

With that having been said, if the size of the name element is "fixed", I'm more OK with what you've done, as that's less likely to encounter the situation where we don't show a tooltip for overflowing text (assuming we have the right length).

[Devin Rousso]

Comment on attachment 363602 [details]
[PATCH] Proposed Fix

r=me

[Joseph Pecoraro]

https://trac.webkit.org/changeset/243026/webkit

[Radar WebKit Bug Importer]

<rdar://problem/48944451>