Closed
Bug 85173
Opened 24 years ago
Closed 23 years ago
Save As... zipfile download with very-long url generates oversized save window
Categories
(SeaMonkey :: UI Design, defect, P3)
SeaMonkey
UI Design
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: fcassia, Assigned: dean_tessman)
References
()
Details
(Whiteboard: fix in hand, need sr=)
Attachments
(8 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
bugzilla
:
review+
bugzilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
With Mozilla 0.91, log into www.idrive.com. Now enter any public shared space (I used username "Maritxa1998" which I found on the Net). Idrive returns a list of publicly shared files. When you click on one of these filenames and start a download, the webserver sends the file with a dynamically generated long url like: www.idrive.com/[verylongstringofchars_sometimes_above_100_chars]/filename.zip Mozilla 0.91 displays the "file download action" window prompting the user what to do with this file that the webserver is sending, displaying the FULL URL, making this window several times the size of the screen. The window is so long, that the "OK" "CANCEL" etc buttons cannot be seen because they end off-screen. I have a screenshot showing this bug. I would attach it but I've just realized that webzilla has no way to attach a file to document visual bugs. (Hint, hint!!).
Reporter | ||
Comment 1•24 years ago
|
||
Reporter | ||
Comment 2•24 years ago
|
||
Comment 3•24 years ago
|
||
confirming with win2k build 20010611.. (CVS opt)
Assignee: asa → pchen
Status: UNCONFIRMED → NEW
Component: Browser-General → XP Apps
Ever confirmed: true
QA Contact: doronr → sairuh
Comment 6•24 years ago
|
||
Updating Platform and OS to all (my testing and bug 84741 confirm it is a problem on Mac too)
OS: Windows 98 → All
Hardware: PC → All
Comment 7•24 years ago
|
||
could someone provide a test username/passwd i could use at idrive.com to test this? thx!
Comment 8•24 years ago
|
||
thanks to Fernando for the clarification: i needed to put "maritxa1998" into the "Visitors" field and click the Visit button. d'oh!
Comment 9•24 years ago
|
||
i also see this on linux --in fact i cannot even dismiss the dialog [well, keyboard nav is busted, bug 74012].
Keywords: mozilla0.9.2
Comment 10•24 years ago
|
||
[sidenote: was able to dismiss by tabbing around the dialog, hitting spacebar, then cancelling if i got the Save File file picker --ie, if i didn't manage tab to the cancel button in the download/helper app dialog.]
Comment 11•24 years ago
|
||
excellent suggestion from Fernando, via email: I think, imho, that a good way to fix this bug is to trim the url displayed to say 50 chars, and add "(...)" at the end. For example showing: Downloading file: http://www.idrive.com/maritxa1998/adskdhskjdhksdhksdsk(...)file.zip
Comment 12•24 years ago
|
||
The same thought had occurred to me. Here's a patch that does that (actually, it replaces the middle with "...", not the end, because usually the end is more interesting): Index: embedding/components/ui/helperAppDlg/nsHelperAppDlg.js =================================================================== RCS file: /cvsroot/mozilla/embedding/components/ui/helperAppDlg/nsHelperAppDlg.js,v retrieving revision 1.9 diff -u -r1.9 nsHelperAppDlg.js --- nsHelperAppDlg.js 2001/05/23 06:03:58 1.9 +++ nsHelperAppDlg.js 2001/06/13 20:01:46 @@ -243,7 +243,16 @@ modified = this.getString( "intro.noDesc" ); } modified = this.replaceInsert( modified, 2, this.mLauncher.MIMEInfo.MIMEType ); - modified = this.replaceInsert( modified, 3, this.mSourcePath ); + var path = this.mSourcePath; + const maxPath = 50; + // Maximum is 50 characters, plus 3 for the "...". + if ( path.length > maxPath + 3 ) { + // Path too long, replace middle section with "...". + path = path.substring( 0, maxPath/2 ) + + "..." + + path.substring( path.length - maxPath/2 ); + } + modified = this.replaceInsert( modified, 3, path ); intro.firstChild.nodeValue = ""; intro.firstChild.nodeValue = modified; },
Comment 13•24 years ago
|
||
nav triage team: Oi, that's a long dialog. Marking nsbeta1+, p3, mozilla0.9.3
Comment 14•24 years ago
|
||
Dean Tessman has a patch to make crop="middle" working, so we could also use that, but since that would crop the entire label, this patch seems fine to me. Since Bill's on sabbatical, I'll taek it and get it in.
Assignee: law → blake
Updated•24 years ago
|
Status: NEW → ASSIGNED
Whiteboard: fix in hand
Target Milestone: mozilla0.9.3 → mozilla0.9.2
Comment 15•24 years ago
|
||
Oops...Bill is still checking in, he must be around :-) Anyways, I'm thinking maybe we should use crop="middle" after all. This is a little hackish, and I'm also not sure if hardcoding "..." like that is a localization problem.
Comment 16•24 years ago
|
||
yes hard coding like that would lead to a localization nightmare. please use crop="middle".
Comment 17•24 years ago
|
||
Sounds like a good idea, so long as it works. I didn't even know about crop="middle" so it's not like I'm partial to coding it up myself. And this is that last Bugzilla comment I'm going to make for a while, anyway.
Comment 18•24 years ago
|
||
Another advantage to using crop="middle" (assuming it works) is that it avoids hard-coding the number of characters to be displayed in the download dialog.
Comment 20•24 years ago
|
||
Sure, it's waiting on 50833 anyways...
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Assignee | ||
Comment 21•24 years ago
|
||
50833 was supposed to get in for 0.9.2, but I don't think that's going to happen. It has an r= and sr= and I mailed drivers for approval but I haven't heard anything back.
Assignee | ||
Comment 23•24 years ago
|
||
I've got crop="middle" working in my tree, using my patch for bug 50833. Can someone save me the time hunting around and suggest where I would put crop="middle" in this dialog to test if it addresses the problem? And why is this marked dependent on bug 74012? That bug is a keyboard problem, nothing to do with this one.
Updated•24 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Reporter | ||
Comment 25•23 years ago
|
||
Well, it seems that Idrive.com closed down their public, free service, where this bug was 1st found. So... how can we test this bug?. I'm sure there are plenty of other sites that generate download file urls "on the fly" with long strings... probably Lotus Notes/Domino based servers, which are famous for the long dynamic URLs. Anyone knows any such site?
Comment 26•23 years ago
|
||
I created a long URL to test this myself. You can find it at http://www.island.net/~rogersd/mozilla/foo/bar/baz/alpha/beta/gamma/delta/test/test/test/long/url/to/break/download/window/notreallya.exe Also, another good test is downloading a file from extra.newsguy.com, as they use some really long URLs to identify the Usenet articles.
Assignee | ||
Comment 27•23 years ago
|
||
There could be a little problem here. I'm not strong in xul, but it seems that the location field is a textbox with a class of scrollbox. Neither of these support the crop attribute. http://lxr.mozilla.org/seamonkey/source/xpfe/components/ucth/resources/helperApp DldProgress.xul#75
Assignee | ||
Comment 29•23 years ago
|
||
Assignee | ||
Comment 30•23 years ago
|
||
*sigh* I just realized I've been working with the wrong dialog. But the patch I just attached does crop the filenames in the download progress dialog! Sorry for all the spam tonight everyone. I blame it on too much sun.
Assignee | ||
Comment 32•23 years ago
|
||
Patch coming to fix this. I split the location into a separate <text> so that it can use crop="center". The odd thing is, I couldn't get location.value = pathString; to work, I had to use location.setAttribute("value", pathString); As jag pointed out to me, in the bindings setting value should just call setAttribute. Odd. http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/t ext.xml#14
Assignee | ||
Comment 33•23 years ago
|
||
Assignee | ||
Comment 34•23 years ago
|
||
Assignee | ||
Comment 35•23 years ago
|
||
Comment 36•23 years ago
|
||
Hrm. So actually that <text> should be a <label> (<text> is deprecated), and apparently we're only putting the xbl bindings on <label>, not on <text>. Not sure if that was intentional or not.
Comment 37•23 years ago
|
||
Note that the dorky construct: intro.firstChild.nodeValue = ""; intro.firstChild.nodeValue = modified; is actually a "highly sophisticated kludge" for a problem with that dialog on Linux (I can't for the life of me find the original bug report for that, but, IIRC, it had to do with the contents shifting and not being repainted when you opened the 'Choose...' dialog). It may be the case that your change makes that no-op line unneeded (or conversely, that we will need to come up with another dumb hack :-\).
Assignee | ||
Comment 38•23 years ago
|
||
Assignee | ||
Comment 39•23 years ago
|
||
New patch using <description> instead of <text>. I sent this off to jag but I
realize he's pretty busy. My build is pre-<description>, can someone test this?
I realize now, after I've attached it, that I still don't use .value. Argh, I
shouldn't do this at 2am - I'm really making a mess of this bug! Before
testing, change location.setAttribute(..) to:
location.value = pathString;
Does the screenshot still look the same as attachment 45740 [details]? I'm wondering
mostly about the cropping, of course, but also that the url lines up nicely on
the left with the other text.
Comment 40•23 years ago
|
||
Yeah, looks okay. So changes I made was to remove comments and replace .setAttribute("value", ...) with .value = ....; And secondly I needed to add value="" to the <description> because otherwise it wouldn't size the element correctly (namely 0 height -> no text visible after setting value). Bug?
Assignee | ||
Comment 41•23 years ago
|
||
I'd say that's a bug, yeah. Sounds like what jrgm was referring to yesterday. I'll do up a new patch tonight with those changes.
Assignee | ||
Comment 42•23 years ago
|
||
Comment 46•23 years ago
|
||
Comment on attachment 45856 [details] [diff] [review] here it is a=asa on behalf of drivers
Attachment #45856 -
Flags: approval+
Assignee | ||
Comment 47•23 years ago
|
||
need an sr=... Blake?
Keywords: mozilla0.9.2
Whiteboard: fix in hand → fix in hand, need sr=
Comment 49•23 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•23 years ago
|
Attachment #45856 -
Flags: superreview+
Attachment #45856 -
Flags: review+
Comment 51•23 years ago
|
||
vrfy fixed. linux, 2001.09.04.08-comm winnt, 2001.09.04.00-comm mac 9.1, 2001.09.04.11-comm
Status: RESOLVED → VERIFIED
Comment 52•23 years ago
|
||
Ok, the window size is fine now (Build 2001090408), but now it doesn't prompt me for a location to save the file when the mime type is set to "Save to disk". It just starts saving it to /tmp and the original filename is lost. However, if it asks "What should Mozilla do with this file?" and I choose "Save this file to disk", it works fine. This didn't happen in 0.9.3
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•