Closed Bug 828302 Opened 7 years ago Closed 7 years ago

Long download file names are not properly cropped

Categories

(Firefox :: Downloads Panel, defect, P2)

x86_64
Windows 7
defect

Tracking

()

VERIFIED FIXED
Firefox 21
Tracking Status
firefox20 --- verified

People

(Reporter: mak, Assigned: mconley)

References

Details

(Keywords: polish, Whiteboard: [testday-20130222])

Attachments

(2 files, 2 obsolete files)

Attached image sshot
This is what I saw today, the progress bar should go to end, but ends earlier, may be a recent regression
Priority: -- → P2
and Optimizer made me notice the problem is actually the long download name is pushing width of the panel, it should be cropped
Summary: Progressbar of "Other downloads" looks wrong → Long download file names are not properly cropped
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.
Attached patch Patch v1 (obsolete) — Splinter Review
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.
Assignee: nobody → mconley
Comment on attachment 699823 [details] [diff] [review]
Patch v1

Fixes the problem for me locally.
Attachment #699823 - Flags: review?(mak77)
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
Attachment #699823 - Flags: review?(mak77)
Attached patch Patch v2 (obsolete) — Splinter Review
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.
Attachment #699823 - Attachment is obsolete: true
Comment on attachment 700006 [details] [diff] [review]
Patch v2

How about this?
Attachment #700006 - Flags: review?(mak77)
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
Attachment #700006 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #8)
> And I'd suggest adding a brief comment above

here I meant "above both rules"
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 added, and using calc now.
Attachment #700006 - Attachment is obsolete: true
Duplicate of this bug: 810479
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
Attachment #700400 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b4b36e2c63ee
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Attachment #700400 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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)
Status: RESOLVED → VERIFIED
Whiteboard: [testday-20130222]
(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.
You need to log in before you can comment on or make changes to this bug.