Closed Bug 238571 Opened 21 years ago Closed 20 years ago

Awkward UI text for showing nonexistent files

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
trivial

Tracking

()

VERIFIED FIXED

People

(Reporter: Waldo, Assigned: bugs)

References

Details

(Keywords: fixed-aviary1.0, Whiteboard: [have patch])

Attachments

(3 files, 3 obsolete files)

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. ;-)
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)
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)?
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.
Attached patch Patch (obsolete) (deleted) — Splinter Review
Thanks Tim...I think this is right now.
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 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-
Attached patch patch v1.1 (obsolete) (deleted) — Splinter Review
This one looks to be a little better...addressing comments from mconnor.
Attachment #151185 - Attachment is obsolete: true
Comment on attachment 151424 [details] [diff] [review] patch v1.1 Trying again, addressing comments from previous review.
Attachment #151424 - Flags: review?(mconnor)
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.
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)
Attached patch Patch v1.2 (obsolete) (deleted) — Splinter Review
Here is the new less personal patch.
Attachment #151424 - Attachment is obsolete: true
Attachment #151424 - Flags: review?(mconnor)
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 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-
No longer blocks: 214267
Blocks: 214267
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?
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]
Attached patch patch, uses Jeff Walden's text (deleted) — Splinter Review
Attachment #151696 - Attachment is obsolete: true
Fixed branch and trunk
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Why not save the bytes and toast the platform-specific strings altogether?
(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.
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 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?
I filed bug 256284 for the string consolidation.
Attachment #156436 - Flags: approval-aviary?
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?
reopening to improve chances of getting attention...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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)
fixed branch and trunk.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
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)
Keywords: fixed-aviary1.0
verified via cvs
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: