1313067 - Text is more truncated with ellipsis in datalist for input

Status: RESOLVED FIXED

(Toolkit :: Autocomplete, defect, P1)

Description

[blinky]

Steps to reproduce:

1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/phacpjwi7i.html.
2. Click on the <input> element to open.


Actual results:

Text is more truncated with ellipsis in datalist for input.

Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/fe9mwb3fst.png

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cf7649531c81d88c1948c88512b0c8c50cf0f2e3&tochange=4b2fef1aa33e310f577dbe36a6c6abf3f1c7f0ce

Regressed by: bug 1296638

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Test case

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Thanks. It looks like the richlist autocomplete binding has some hard-coded extra padding at the end of each richlistitem.

I'll make that opt-in on the binding.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1313067 - Make extra padding in autocomplete popup richlistitem opt-in.

Review commit: https://reviewboard.mozilla.org/r/89354/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89354/

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8805602 [details]
Bug 1313067 - Make extra padding in autocomplete popup richlistitem opt-in.

https://reviewboard.mozilla.org/r/89354/#review90090

::: browser/base/content/browser.xul:156
(Diff revision 1)
>             id="PopupAutoCompleteRichResult"
>             noautofocus="true"
>             hidden="true"
>             flip="none"
> -           level="parent"/>
> +           level="parent"
> +           extrapadding="true" />

ah, I didn't remember of this, my memory gets rusty.

What about naming this overflowpadding and making it a numeric value rather than a boolean, so:
overflowpadding="30"

::: toolkit/content/widgets/autocomplete.xml:1411
(Diff revision 1)
>            ]]>
>          </setter>
>        </property>
>  
> +      <property name="useExtraPadding"
> +                onget="return this.getAttribute('extrapadding') == 'true';"

and here you could return 0 as the default value, so below you can just use popup.overflowPadding in the calculation and avoid assigning to a pointless extraPadding temp var.

using Number(...getAttribute()) may do the trick (empty string is handled as 0).

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8805602 [details]
Bug 1313067 - Make extra padding in autocomplete popup richlistitem opt-in.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89354/diff/1-2/

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8805602 [details]
Bug 1313067 - Make extra padding in autocomplete popup richlistitem opt-in.

https://reviewboard.mozilla.org/r/89354/#review90144

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b30232f430a
Make extra padding in autocomplete popup richlistitem opt-in. r=mak

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4b30232f430a