828302 - Long download file names are not properly cropped

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P2)

Description

[Marco Bonardo [:mak]]

Attachment: sshot

This is what I saw today, the progress bar should go to end, but ends earlier, may be a recent regression

Comment 1

[Marco Bonardo [:mak]]

and Optimizer made me notice the problem is actually the long download name is pushing width of the panel, it should be cropped

Comment 2

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

So, the problem here is that we're setting a min-width on the description, but since there's no maximum, the description doesn't know when to start cropping.

The solution is to use width instead of min-width.

Comment 3

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

Attachment: Patch v1

So, here's a refinement to my solution: I set a width on the downloadTarget to match downloadDetails.

min-width overrides width, so in the event that some locales have the downloadDetails.width < downloadsSummary.minWidth2, we automatically set the width to the larger of the two. This allows cropping to work properly.

Comment 4

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

Comment on attachment 699823 [details] [diff] [review]
Patch v1

Fixes the problem for me locally.

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 699823 [details] [diff] [review]
Patch v1

Review of attachment 699823 [details] [diff] [review]:
-----------------------------------------------------------------

as discussed over IRC, this increases size of the panel cause the target font is larger than the details font

Comment 6

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

Attachment: Patch v2

Ok, alternative solution; set the downloadDetails width on the vbox that wraps downloadTarget, downloadProgress and downloadDetails.

It means setting downloadDetails' font-size percentage on the vbox, and then boosting downloadTarget's font-size. It's not totally precise, but the error is negligible.

Comment 7

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

Comment on attachment 700006 [details] [diff] [review]
Patch v2

How about this?

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 700006 [details] [diff] [review]
Patch v2

Review of attachment 700006 [details] [diff] [review]:
-----------------------------------------------------------------

I'm really glad you found this solution

::: browser/themes/gnomestripe/downloads/downloads.css
@@ +104,2 @@
>  .downloadTarget {
> +  font-size: 111%;

mbrubeck in #fx-team suggested calc(100%/0.9) that is easier to mentally connect to the above 90%, or in the pinstripe case to 95% (so no need to recalculate manually if it should ever change)...

may be a bit more expensive but in the end we just show 3 items here.

And I'd suggest adding a brief comment above explaining why we do this (included a reference to &downloadDetails.width;).

same across all themes

Comment 9

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [:mak] from comment #8)
> And I'd suggest adding a brief comment above

here I meant "above both rules"

Comment 10

[Marco Bonardo [:mak]]

Actually, the details text can also contain the host, and the host may be long, so please reintroduce crop="end" in the details field. sorry for misinformation.

Comment 11

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

Attachment: Patch v3 (r+'d by mak)

Comment added, and using calc now.

Comment 12

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

Landed on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b36e2c63ee

Comment 14

[Marco Bonardo [:mak]]

Comment on attachment 700400 [details] [diff] [review]
Patch v3 (r+'d by mak)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Downloads panel feature
User impact if declined: incomplete ui
Testing completed (on m-c, etc.): m-i (merge pending)
Risk to taking this patch (and alternatives if risky): limited to the feature
String or UUID changes made by this patch: none

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b4b36e2c63ee

Comment 16

[Marco Bonardo [:mak]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/57f1157c4b45

Comment 17

[Mihai Morar, (:MihaiMorar)]

I confirm the fix is verified on FF19b1 and Latest Nightly (2013-02-21) on Windows 7.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0(20130220104816)

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130109 Firefox/21.0(20130109030942)

Comment 18

[Mihai Morar, (:MihaiMorar)]

(In reply to MarioMi (:MarioMi) from comment #17)
 Verified fixed on FF 20.b1 not FF 19.b1, just worked a lot on Beta 19 in last weeks and used to write 19.