915854 - Watch expressions should wrap to show result

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Timm Rebitzki]

Attachment: Screen shot 2013-09-12 at 22.23.05 .png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130814063812

Steps to reproduce:

Add a watch expression.
Stop on a breakpoint.


Actual results:

The watch expression is highlighted with an arrow next to it. The panel is too narrow to show the result next to the arrow.


Expected results:

I expect the result always to be visible. Obviously the panel is too narrow (see screenshot), but it is not very efficient to resize it just so I can see the result.

The expression and its value should be wrapped to as many lines as necessary to show the whole result.

This may in some cases also be a UI issue, because it looks like a value should appear if I click a reveal button, but in reality what I want to see is just in the same line out of view. This is frustating; I'm only filing a bug because I happened to see part of the result when I maximized the window!

Comment 1

[Sebastian Zartner [:sebo]]

This is still an issue in the new Debugger frontend. Tested in Nightly 57.0a1 (2017-08-17).

Sebastian

Comment 2

[Kevin Morehouse]

I'll gladly pick this up.

Comment 3

[Kevin Morehouse]

Attachment: FirefoxWatchExpressionsBreakWord.mov

I've used the CSS editor to create a working demo of what this feature would look like.

When this bug was originally filed, the proposed solution is that watch expressions "should be wrapped to as many lines as necessary to show the whole result."

While this is desirable in theory, it creates UI problems in practice. As the panel narrows, the wrapping of the expression would force the panel to lengthen vertically.

In the video, you can it would look like for string expressions if overflow-wrap was set to break-word. The width of the string will decrease until it reaches the width of the narrowest word in the string.

Comment 4

[Kevin Morehouse]

the width of the narrowest word in the string.

I meant widest word in the string, not narrowest.

Comment 5

[Kevin Morehouse]

Attachment: FirefoxWatchExpressionsBreakAnywhere.mov

The situation worsens if overflow-wrap is set to anywhere, as the expression width will decrease all the way to the width of a single character, potentially making the panel even taller than it was before.

Even if we use break-word for strings (as described in my previous comment), this is the solution we would have to implement for numbers, as they can't break into words.

Comment 6

[Kevin Morehouse]

Attachment: ChromeWatchExpressionsBehavior.mov

As an alternative solution, I would like to suggest that we seek parity with Chrome, and avoid wrapping watch expressions altogether.

Under Chrome's implementation, watch expressions are not wrapped when the right sidebar is resized. To see the whole expression when the panel is narrowed, users have the option to hover over the expression and view the entire expression as a tooltip.

Comment 7

[Kevin Morehouse]

Attachment: FirefoxWatchExpressionsBehavior.mov

Our current implementation is very close to Chrome's. The only difference is that our tooltip on hover displays the property name, not the property value.

To resolve this bug, I would like to fix the tooltip to display the property value. I spoke with David Walsh during the debugger community call today, and he also agreed this would be a worthwhile solution.

Comment 8

[Kevin Morehouse]

Attachment: Bug 915854 - Work-in-Progress Patch - Add detailed tooltips and ellipses to watch expressions. r=davidwalsh

This patch addresses issues discussed on bug 915854. It modifies the tooltip of watch expressions in the watch expressions panel to display information related to the value of that watch expression, rather than the key. It also appends ellipses to the end of watch expressions values that have been cut off by the width of the watch expressions panel. Furthermore, it removes a secondary bug that was forcing symbol watch expressions to wrap in the watch expressions pane.

Comment 9

[Sebastian Zartner [:sebo]]

One note, in your last video the value of the expression was overlayed by the x to remove the expression. So, that needs to be fixed if not already done. The simplest solution is probably to add a display: flex; to the container of the expression.

Sebastian

Comment 10

[Kevin Morehouse]

(In reply to Sebastian Zartner [:sebo] from comment #9)

One note, in your last video the value of the expression was overlayed by the x to remove the expression. So, that needs to be fixed if not already done. The simplest solution is probably to add a display: flex; to the container of the expression.

Sebastian

Hi Sebastian!

I just built the tip of the default branch, and found that the UI bug you mentioned is still present. I'll fix it in my feature build and add it to my WIP patch. Thank you for the feedback!

Comment 11

[Patrick Brosset <:pbro>]

Hi Kevin. I'm just checking if you are still interested in working on this bug. There hasn't been any activity here or on the patch in phabricator for some time. If you just need more time, that's fine. If you, however, are not planning on working on this bug anymore, let us know so it can be made available for others to pick up.

Comment 12

[Andrei Petcu]

Can I work on this? It seems that Kevin is not interested in it anymore.

Comment 13

[:Harald Kirschner :digitarald]

It's yours. Thanks a lot for your help! Feel free to ask questions in Slack or here if you need to ramp up.

Comment 14

[Patrick Brosset <:pbro>]

Hi petcuandrei, I'm just checking in to see how you're doing. Do you need any help with the work on this? Do you still intend to work on this bug?

Comment 15

[Patrick Brosset <:pbro>]

I'm going to unassign this bug for now since petcuandrei did not answer. Maybe someone else wants to pick up from here.

Comment 16

[Andrei Petcu]

Sorry for the late repply. I cannot work on this :(

Comment 17

[Victor Akinyemi :tehlordvortex]

Hello!

I am new to contributing to the Firefox DevTools and would like to work on this.

Comment 18

[Richard Sherman :rich :richorrichard]

Gah! beat me to the punch! @tehlordvortex - if you decide to work on another bug id love to take this one. also looking for my first bug to work on, and I know Kevin, who did some of the work previously on this bug.

Comment 19

[Victor Akinyemi :tehlordvortex]

@rich sure, no problem! I'll pick up another bug ;)

Comment 20

[Richard Sherman :rich :richorrichard]

Oh thanks - didn't expect that! I appreciate your flexibility Victor! Harold or Patrick - can i claim this as my first bug?

Comment 21

[:Harald Kirschner :digitarald]

Thanks :rich. Feel free to ask questions here or in our team chat's #debugger channel.

Comment 22

[Richard Sherman :rich :richorrichard]

Thanks! Will do!

Comment 23

[Richard Sherman :rich :richorrichard]

:digitarald and :jlast I've put together a quick POC of my proposed solution. I can also commit my changes for discussion (I may need help with the workflow for this first time through for getting it up to phabricator). My solution provides a native title tooltip for primitive data types rendered using the ObjectInspector component. I've also noticed that the original bug's solve for trimming the text in a long string does not currently work down in the scopes pane, so that may be worth looking into as well to ensure that the end functionality is the same for each place that uses the ObjectInspector component.

If this does align with desired functionality I'd like to discuss in more detail some of the considerations we need to make for extensibility - may be worth having that conversation in slack and/or a google doc due to the number of files involved.

POC video:
https://www.loom.com/share/2b4477a61849475eae8debb803704285

Comment 24

[Richard Sherman :rich :richorrichard]

:digitarald and :jlast I have a suggestion that we mark this bug as complete, since Kevin's work before me addressed truncating long values and exposing the 'remove watch expression' x button.

Kevin made a recommendation that we also include some form of more helpful context tooltips for those longer values, and since most of the work I've been doing is on these tooltip titles, and it seems too unrelated to this particular bug to keep it housed here, I've created another bug to address the larger issue at hand. https://bugzilla.mozilla.org/show_bug.cgi?id=1631545. Please advise!

Comment 25

[:Harald Kirschner :digitarald]

Sorry for the delay, :rich! Closing this makes sense, thanks a lot for looking it – looking forward to see your work land for bug 1631545!