Closed
Bug 238571
Opened 21 years ago
Closed 20 years ago
Awkward UI text for showing nonexistent files
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
VERIFIED
FIXED
People
(Reporter: Waldo, Assigned: bugs)
References
Details
(Keywords: fixed-aviary1.0, Whiteboard: [have patch])
Attachments
(3 files, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
The text currently is:
<file> does not exist. (Perhaps you moved it from the place it was downloaded
to?). Because of this, Firefox cannot show this file in Explorer.
Issues:
1. From a nitpicky grammatical point, the parenthesized sentence ends with a
preposition. Firefox UI sentences shouldn't end with a preposition unless the
alternative sounds incredibly unnatural (and the existing string already sounds
unnatural).
2. Doubled punctuation of the second sentence (once inside parentheses
correctly, once outside incorrectly)
3. "Because of this" might be correct, but I don't think it is. One issue to
note is that it's *usually* bad form to start a sentence with "Because".
My humble suggestion:
<file> does not exist, so Firefox cannot show this file. It may have been moved
or deleted since you downloaded it.
It's not perfect, but it's an improvement. Perhaps someone else can find a
string that works better? (I hope it's okay that I added this to the bug 214267
blockers list; I saw another bug blocking the 1.0 Features bug with blocking
bugs added by non-official testers, so I figured it was okay if I did so for
this bug as well.)
<file> does not exist and cannot be shown. Perhaps the file was manually
renamed, moved, or deleted after it was downloaded.
"manually" shifts blame away from Firefox. ;-)
Comment 2•21 years ago
|
||
I have changed the text in my downloads.properties file. I do not know how to
make a patch, or I would attach it here (it is very simple, only 4 lines changed)
Comment 3•21 years ago
|
||
If I do a diff command from a stock version of downloads.properties to the one I
modified, this is what I get:
---Begin---
--- downloads.properties 2004-05-27 16:15:12.000000000 -0400
+++
/home/neil/downloads/firefox-source/mozilla/toolkit/mozapps/downloads/locale/downloads.properties
2004-05-27 16:06:10.000000000 -0400
@@ -1,4 +1,3 @@
-
transferred=%1SKB of %2SKB
downloading=Downloading
notStarted=Not Started
@@ -29,11 +28,11 @@
shortTimeFormat=#2:#3
fileDoesNotExistOpenTitle=Cannot Open %S
-fileDoesNotExistOpenError=%1S does not exist. (Perhaps you moved it from the
place it was downloaded to?). Because of this, %2S cannot open this file.
+fileDoesNotExistOpenError=%1S does not exist and cannot be shown. Perhaps you
renamed, moved, or deleted it after it was downloaded. This error prevents %2S
from displaying this file.
fileDoesNotExistShowTitle=Cannot Show %S
-fileDoesNotExistShowErrorWin=%S does not exist. (Perhaps you moved it from the
place it was downloaded to?). Because of this, %S cannot show this file in Explorer.
-fileDoesNotExistShowErrorMac=%S does not exist. (Perhaps you moved it from the
place it was downloaded to?). Because of this, %S cannot show this file in Finder.
-fileDoesNotExistShowErrorUnix=%S does not exist. (Perhaps you moved it from the
place it was downloaded to?). Because of this, %S cannot show this file in a
browser window.
+fileDoesNotExistShowErrorWin=%S does not exist and cannot be shown. Perhaps you
rename, moved, or deleted it after it was downloaded. This error prevents %S
from displaying this file file in Explorer.
+fileDoesNotExistShowErrorMac=%S does not exist and cannot be shown. Perhaps you
renamed, moved or deleted it after it was downloaded. This error prevents %S
from displaying this file in Finder.
+fileDoesNotExistShowErrorUnix=%S does not exist and cannot be shown. Perhaps
you renamed, moved, or deleted it after it was downloaded. This error prevents
%S from displaying this file in a browser window.
chooseAppFilePickerTitle=Open With...
downloadsTitle=%S%% of 1 file - Downloads
---End---
But that is not the output that I need; how do I get that (the required one)?
Comment 4•20 years ago
|
||
Neil, try using cvs instead...
from the mozilla directory of your cvs tree, issue:
cvs diff -u paths/to/files/patched > name_of_outputfile
This should give you the correct patch format.
Comment 5•20 years ago
|
||
Thanks Tim...I think this is right now.
Comment 6•20 years ago
|
||
Comment on attachment 151185 [details] [diff] [review]
Patch
mconnor, can you review this? Its a 4 line patch that changes text.
Attachment #151185 -
Flags: review?(mconnor)
Comment 7•20 years ago
|
||
Comment on attachment 151185 [details] [diff] [review]
Patch
I'm not sure I like this wording in general. It really seems awkward and
wordy.
The "This error prevents" is just plain wrong. Because of this is better.
Also, "This error prevents %S from" is wrong for another reason, namely that
you'd have something like "This error prevents foo.txt from displaying this
fine in Finder"
Attachment #151185 -
Flags: review?(mconnor) → review-
Comment 8•20 years ago
|
||
This one looks to be a little better...addressing comments from mconnor.
Attachment #151185 -
Attachment is obsolete: true
Comment 9•20 years ago
|
||
Comment on attachment 151424 [details] [diff] [review]
patch v1.1
Trying again, addressing comments from previous review.
Attachment #151424 -
Flags: review?(mconnor)
Comment 10•20 years ago
|
||
Comment on attachment 151424 [details] [diff] [review]
patch v1.1
I don't think I like the personalization of the dialog text here, either, since
someone else could have done it too.
"It may have been renamed, moved, or deleted after it was downloaded." seems
better, but I'm tired and possibly not judging well. I'll look at this
tomorrow.
Comment 11•20 years ago
|
||
Thanks for looking at these patches quickly, Mike. A quick question though: I
seem to be misinterpreting the %S/%2S. %S is referencing the file downloaded, is
that what %2S is too? The "fileDoesNotExistOpenError=" uses %2S while the others
use %S.
Yes, I do realize that maybe I shouldn't be writing a patch if I don't know what
I'm changing, but this does seem to be a simple change. I would try this in a
build of my own, but I can't get any builds to work...
Just in case I have it right though, I have another patch made that changes the
personalization, as per comment #10 (if you decide thats better)
Comment 12•20 years ago
|
||
Here is the new less personal patch.
Attachment #151424 -
Attachment is obsolete: true
Updated•20 years ago
|
Attachment #151424 -
Flags: review?(mconnor)
Comment 13•20 years ago
|
||
Comment on attachment 151696 [details] [diff] [review]
Patch v1.2
Cleared review? on previous patch, asking for it again here.
Attachment #151696 -
Flags: review?(mconnor)
Comment 14•20 years ago
|
||
Comment on attachment 151696 [details] [diff] [review]
Patch v1.2
you want to try using patch maker and unjarring chrome to test stuff if you
can't get CVS builds working.
As for the variables, without even looking at the code, the first one is
%1S/%2S, which would be filename/app that its supposed to be opened with (i.e.
silly.mpg does not exist so Media Player cannot open it.)
The second is just %S, and that's the filename. What you need to do though is
change the first string to the original last sentence, since that is correct.
Attachment #151696 -
Flags: review?(mconnor) → review-
Reporter | ||
Comment 15•20 years ago
|
||
This is ugly enough that I'm nominating for blocking-aviary1.0PR, as the tracker
has been minused.
What text string is wanted here? I'll make a patch ASAP when I know, but from
the comments I can't tell if any decision has been reached.
My suggestions:
<file> does not exist. It may have been renamed, moved, or deleted.
-or-
<file> does not exist. It may have been renamed, moved, or deleted since it was
downloaded.
Flags: blocking-aviary1.0PR?
Comment 16•20 years ago
|
||
I'd vote for the latter. we would need to get a patch, have it reviewed, and
get it checked in this week if it is going to make PR and 1.0 at this point.
Flags: blocking-aviary1.0PR? → blocking-aviary1.0PR+
Whiteboard: [have patch]
Assignee | ||
Comment 17•20 years ago
|
||
Attachment #151696 -
Attachment is obsolete: true
Assignee | ||
Comment 18•20 years ago
|
||
Fixed branch and trunk
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 19•20 years ago
|
||
Why not save the bytes and toast the platform-specific strings altogether?
Comment 20•20 years ago
|
||
(In reply to comment #19)
> Why not save the bytes and toast the platform-specific strings altogether?
Okay. This option removes the platform-specifics but leaves the
fileDoesNotExist strings for opening and showing separate.
The next patch is for making a fileDoesNotExistError string that's shared by
both dialogs.
Reporter | ||
Comment 21•20 years ago
|
||
The original bug's fixed, but this is some followup. It would need to get in
before 1.0PR if it's to get in before 1.0. I don't know if I need to reopen
the bug for this or not, or whether I should just renominate it for
blocking-aviary1.0PR. Someone else more knowledgeable about the correct
procedures can take care of that.
Comment 22•20 years ago
|
||
Comment on attachment 156436 [details] [diff] [review]
Option #2: create one error string for both dialogs and use it
I like this approach. If for some reason we want to split the strings out
later, we can add in a second string again.
Mike or Ben, any chance of getting this in for PR1 to save a few bytes?
Comment 23•20 years ago
|
||
I filed bug 256284 for the string consolidation.
Updated•20 years ago
|
Attachment #156436 -
Flags: approval-aviary?
Comment 24•20 years ago
|
||
Comment on attachment 156436 [details] [diff] [review]
Option #2: create one error string for both dialogs and use it
please don't request approval until you have a fully reviewed patch. thanks.
Attachment #156436 -
Flags: approval-aviary?
Comment 25•20 years ago
|
||
reopening to improve chances of getting attention...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 26•20 years ago
|
||
Comment on attachment 156436 [details] [diff] [review]
Option #2: create one error string for both dialogs and use it
some error string changes needing review - chucking at mconnor for
review-triage :)
Attachment #156436 -
Flags: review?(mconnor)
Assignee | ||
Comment 27•20 years ago
|
||
fixed branch and trunk.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Comment 28•20 years ago
|
||
Comment on attachment 156436 [details] [diff] [review]
Option #2: create one error string for both dialogs and use it
this is the patch that Ben checked in (with r+a=ben in the comment)
Attachment #156436 -
Flags: review?(mconnor)
Updated•20 years ago
|
Keywords: fixed-aviary1.0
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•