Closed
Bug 393303
Opened 17 years ago
Closed 17 years ago
Create a blocked string for dirty downloads (virus scan failed)
Categories
(Toolkit :: Downloads API, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla1.9beta2
People
(Reporter: robarnold, Assigned: sdwilsh)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file, 1 obsolete file)
17.95 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
This is now used for both parental controls and downloads with uncleanable viruses
Updated•17 years ago
|
Comment 1•17 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=103487#c54 appears to be what this bug was spun off from. I kinda suspect that splitting this into two states might be right thing to do, rather than just switching the string, which is currently "Blocked by Parental Controls".
This seems guaranteed to be confusing to the end-user; requesting blocking-1.9.
Flags: blocking-firefox3?
Comment 2•17 years ago
|
||
(In reply to comment #1)
> bug was spun off from. I kinda suspect that splitting this into two states
> might be right thing to do, rather than just switching the string, which is
> currently "Blocked by Parental Controls".
I would agree. One case is "you have decided to not show this content" and the other case is "this content has been deemed unsafe by your AV software", which shouldn't be handled by the same user facing message.
Flags: blocking-firefox3? → blocking-firefox3+
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 M10
Assignee | ||
Comment 3•17 years ago
|
||
Morphing - we need a new string though.
I'll make the new state DOWNLOAD_DIRTY, but that has no affect on the string presented to the user.
Madhava - suggestions for strings?
Assignee: nobody → comrade693+bmo
No longer depends on: 400617
Summary: Make blocked string more generic → Create a blocked string for dirty downloads (virus scan failed).
Assignee | ||
Updated•17 years ago
|
Whiteboard: [needs patch]
Assignee | ||
Comment 4•17 years ago
|
||
How about "Blocked by third-party file scanner"?
Comment 5•17 years ago
|
||
How about even more direct?
"Blocked: may contain virus or spyware"
Assignee | ||
Comment 6•17 years ago
|
||
Stephen - can you comment on the grammar of this please.
Assignee | ||
Comment 7•17 years ago
|
||
This should do the trick. I can't test that it works since my windows build is broken still, but I did test that it doesn't break things on my mac.
Gavin, could you test this when you review it please?
Attachment #289691 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [needs patch] → [has patch][needs review gavin]
Comment 8•17 years ago
|
||
Strictly speaking, "may" is for permission-oriented verbiage, while "might" is for possibilities; vernacular allows for either, but I'd prefer "might".
Comment 9•17 years ago
|
||
Comment on attachment 289691 [details] [diff] [review]
v1.0
>Index: toolkit/components/downloads/src/nsDownloadManager.cpp
> nsDownloadManager::CleanUp()
>- for (PRUint32 i = 0; i < 4; ++i) {
>+ for (PRUint32 i = 0; i < 5; ++i) {
Use NS_ARRAY_LENGTH(states)?
> nsDownloadManager::GetCanCleanUp(PRBool *aResult)
>- "OR state = ?4"), getter_AddRefs(stmt));
>+ "OR state = ?4 "
>+ "OR state = ?5"), getter_AddRefs(stmt));
> NS_ENSURE_SUCCESS(rv, rv);
> for (PRUint32 i = 0; i < 4; ++i) {
Here too (forgot to change the 4).
Don't you also need to update removable and canceledOrFailed on download-base in download.xml (although it appears that canceledOrFailed is no longer in use)? And onDownloadStateChange in DownloadProgressListener.js?
>Index: toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties
> stateBlocked=Blocked by Parental Controls
>+stateDirty=Blocked: may contain virus or spyware
Having the colon in one of the "blocked" strings but not the other seems kind of odd, but I guess it doesn't much matter. I can't think of a better way of putting it.
Patch tests out fine otherwise.
Attachment #289691 -
Flags: review?(gavin.sharp) → review-
Comment 10•17 years ago
|
||
Comment on attachment 289691 [details] [diff] [review]
v1.0
>+stateDirty=Blocked: may contain virus or spyware
Capitalize "may" here. Maybe even "Blocked: Download may contain a virus or spyware".
However, stephend made a good point. I did some research, and common use seems to be a toss-up between may versus might. http://www.economist.com/research/styleGuide/index.cfm?page=673903 makes me want to use "may" here, though.
Assignee | ||
Updated•17 years ago
|
Whiteboard: [has patch][needs review gavin] → [needs new patch]
Assignee | ||
Comment 11•17 years ago
|
||
Addresses review comments, and updates the string accordingly.
Attachment #289691 -
Attachment is obsolete: true
Attachment #290074 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [needs new patch] → [has patch][needs review gavin]
Updated•17 years ago
|
Attachment #290074 -
Flags: review?(gavin.sharp) → review+
Updated•17 years ago
|
Whiteboard: [has patch][needs review gavin] → [has patch]
Updated•17 years ago
|
Whiteboard: [has patch] → [has patch][has review][can land]
Assignee | ||
Comment 12•17 years ago
|
||
I don't forsee myself having time to land this, so...
Keywords: checkin-needed
Comment 13•17 years ago
|
||
Checking in toolkit/components/downloads/public/nsIDownloadManager.idl;
/cvsroot/mozilla/toolkit/components/downloads/public/nsIDownloadManager.idl,v <-- nsIDownloadManager.idl
new revision: 1.23; previous revision: 1.22
done
Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v <-- nsDownloadManager.cpp
new revision: 1.148; previous revision: 1.147
done
Checking in toolkit/components/downloads/src/nsDownloadScanner.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadScanner.cpp,v <-- nsDownloadScanner.cpp
new revision: 1.4; previous revision: 1.3
done
Checking in toolkit/mozapps/downloads/content/DownloadProgressListener.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/DownloadProgressListener.js,v <-- DownloadProgressListener.js
new revision: 1.29; previous revision: 1.28
done
Checking in toolkit/mozapps/downloads/content/download.xml;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/download.xml,v <-- download.xml
new revision: 1.40; previous revision: 1.39
done
Checking in toolkit/mozapps/downloads/content/downloads.css;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.css,v <-- downloads.css
new revision: 1.12; previous revision: 1.11
done
Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js
new revision: 1.106; previous revision: 1.105
done
Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties,v <-- downloads.properties
new revision: 1.15; previous revision: 1.14
done
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: Create a blocked string for dirty downloads (virus scan failed). → Create a blocked string for dirty downloads (virus scan failed)
Whiteboard: [has patch][has review][can land]
Comment 14•17 years ago
|
||
Relanded.
Checking in toolkit/components/downloads/public/nsIDownloadManager.idl;
/cvsroot/mozilla/toolkit/components/downloads/public/nsIDownloadManager.idl,v <-- nsIDownloadManager.idl
new revision: 1.25; previous revision: 1.24
done
Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v <-- nsDownloadManager.cpp
new revision: 1.150; previous revision: 1.149
done
Checking in toolkit/components/downloads/src/nsDownloadScanner.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadScanner.cpp,v <-- nsDownloadScanner.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in toolkit/mozapps/downloads/content/DownloadProgressListener.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/DownloadProgressListener.js,v <-- DownloadProgressListener.js
new revision: 1.31; previous revision: 1.30
done
Checking in toolkit/mozapps/downloads/content/download.xml;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/download.xml,v <-- download.xml
new revision: 1.42; previous revision: 1.41
done
Checking in toolkit/mozapps/downloads/content/downloads.css;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.css,v <-- downloads.css
new revision: 1.14; previous revision: 1.13
done
Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js
new revision: 1.108; previous revision: 1.107
done
Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties,v <-- downloads.properties
new revision: 1.17; previous revision: 1.16
done
Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120705 Minefield/3.0b2pre and AVG Free 7.5.503, with the eicar.zip testfile.
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•