Closed Bug 1470103 Opened 2 years ago Closed 2 years ago

Autocomplete popup width is too small

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: jdescottes, Assigned: nchevobbe)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [boogaloo-mvp])

Attachments

(2 files)

STRs:
- open console
- run `var w = window`
- type `w` then `.` (don't simply paste `w.`)

The autocomplete popup is too narrow to show the suggestions full width. 
It seems linked to the fact that when typing `w` the autocomplete is already displayed for "w" and "window" and doesn't need to have a big width. 

Also occurs on release, not a recent regression.
Let's try to fix this as part of the jsterm improvements
Blocks: 1458831
Whiteboard: [boogaloo-mvp]
Priority: P3 → P2
See Also: → 1461522
Blocks: 1468999
This looks like the combination of 2 things:

1. When calling setItems in the autocomplete popup, we don't call updateSize, which computes the maximum number of chars of a label, and set it as the width of the inner `_list` element. updateSize is only called when calling openPopup, which is not called in the STR from Comment 0.
2. Even if we call updateSize in setItems, it gets overridden by the explicit width the HTMLTooltip is setting (https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/devtools/client/shared/widgets/tooltip/HTMLTooltip.js#545). I guess we should re-trigger updateContainerBounds when setting items as well.
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
The test isn't ideal, but it does fail without the fix in the code.
Comment on attachment 8993669 [details]
Bug 1470103 - Fix autocomplete popup sizing issue in the console; .

removing flag since there are failure in TRY run (https://treeherder.mozilla.org/#/jobs?repo=try&revision=a17f1bd91b96d7230c9aba7264ddf0fcde440e93&selectedJob=189197506)
Attachment #8993669 - Flags: review?(jdescottes)
Comment on attachment 8993669 [details]
Bug 1470103 - Fix autocomplete popup sizing issue in the console; .

https://reviewboard.mozilla.org/r/258346/#review265672

Nice, fixes the issue and glad we have a test.

::: devtools/client/webconsole/components/JSTerm.js:1292
(Diff revision 2)
>        }
>  
>        if (popupAlignElement) {
>          popup.openPopup(popupAlignElement, xOffset, yOffset);
>        }
>      } else if (items.length === 0 && popup.isOpen) {

In theory this could now be `else if (popup.isOpen)` since we know that items is an array, and length is not > 0 ... but I actually think it's better to keep it. Helps parsing the code quickly :)
Attachment #8993669 - Flags: review?(jdescottes) → review+
Comment on attachment 8993669 [details]
Bug 1470103 - Fix autocomplete popup sizing issue in the console; .

https://reviewboard.mozilla.org/r/258346/#review265672

> In theory this could now be `else if (popup.isOpen)` since we know that items is an array, and length is not > 0 ... but I actually think it's better to keep it. Helps parsing the code quickly :)

yes, that was my thought since the if block is a bit "big"
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59adeffe6843
Fix autocomplete popup sizing issue in the console; r=jdescottes.
https://hg.mozilla.org/mozilla-central/rev/59adeffe6843
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.