"attachment" content shows a checkbox for "don't ask"

RESOLVED FIXED in mozilla1.5alpha

Status

Core Graveyard
File Handling
P1
normal
RESOLVED FIXED
15 years ago
2 years ago

People

(Reporter: bz, Assigned: Biesinger)

Tracking

({helpwanted})

Trunk
mozilla1.5alpha
helpwanted

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

And we ignore that checkbox for such content.  We need to come up with a
coherent strategy here.  A thorough description of what IE does and does not do
would be very nice.....
.
Assignee: law → bzbarsky
Keywords: helpwanted
Priority: -- → P2
Target Milestone: --- → mozilla1.5beta
*** Bug 186552 has been marked as a duplicate of this bug. ***
*** Bug 209761 has been marked as a duplicate of this bug. ***
unless my testcase was wrong, MSIE 5.0 and 5.5 (I don't have 6.0 handy
currently) both show the normal "Open"/"Save" dialog but with a greyed-out "[x]
always ask" checkbox.

How about we just do the same?
That's exactly what we should be doing... except we should also change the text
to say something like "The site has requested that this content be handled
externally" instead of "we don't know how to handle this content".
ok. taking.
Assignee: bzbarsky → cbiesinger
Priority: P2 → P1
Target Milestone: mozilla1.5beta → mozilla1.5alpha
Created attachment 126870 [details] [diff] [review]
patch

implement what bz described before.

This will make the checkbox in the dialog not reflect the MIMEInfo value (for
content-disposition:attachment content)
Attachment #126870 - Flags: review?(bzbarsky)
Comment on attachment 126870 [details] [diff] [review]
patch

r=bzbarsky.  you're on a roll, man!
Attachment #126870 - Flags: review?(bzbarsky) → review+
Comment on attachment 126870 [details] [diff] [review]
patch

actually, this patch is not complete, there are other implementations of this
interface that must be changed. will attach an additional patch
Created attachment 126900 [details] [diff] [review]
fix other implementers

this is needed in addition to the other patch
Comment on attachment 126900 [details] [diff] [review]
fix other implementers

this time, including the necessary camino change.
Attachment #126900 - Flags: review?(bzbarsky)
Comment on attachment 126900 [details] [diff] [review]
fix other implementers

r=me; please file bugs on camino and such to update their wording if they want
to.
Attachment #126900 - Flags: review?(bzbarsky) → review+
Attachment #126870 - Flags: superreview?(darin)
Comment on attachment 126900 [details] [diff] [review]
fix other implementers

bz: ok, I'll file those bugs when I checkin this patch
Attachment #126900 - Flags: superreview?(darin)

Comment 14

15 years ago
Comment on attachment 126870 [details] [diff] [review]
patch

>Index: embedding/components/ui/helperAppDlg/nsHelperAppDlg.js

>+        if ( this.mForced && desc )
>+        {
>+          modified = this.replaceInsert( this.getString( "intro.attachment.label" ), 1, desc );
>+        }
>+        else if ( this.mForced && !desc ) {
>+          modified = this.getString( "intro.attachment.noDesc.label" );
>+        }
>+        else if ( desc )
>         {

nit: keep the brace styles consistent ;-)


>Index: embedding/components/ui/helperAppDlg/nsHelperAppDlg.xul

>     <strings style="display:none;">
>         <string id="brandShortName">         &brandShortName;         </string>
>         <string id="intro.withDesc">         &intro.label;            </string>
>         <string id="intro.noDesc">           &intro.noDesc.label;     </string>
>+        <string id="intro.attachment.label"> &intro.attachment.label; </string>
>+        <string id="intro.attachment.noDesc.label">&intro.attachment.noDesc.label;</string>
>         <string id="defaultApp">             &useSystemDefault.label; </string>
>         <string id="badApp">                 &badApp;                 </string>
>         <string id="badApp.title">           &badApp.title;           </string>
>         <string id="chooseAppFilePickerTitle">&chooseAppFilePickerTitle;</string>

nit: the original author of this file seemed to want things lined up.  maybe
you could fix these to be that way when you check in.


>Index: uriloader/exthandler/nsExternalHelperAppService.cpp

>+  if (!mHandlingAttachment) {
>     mMimeInfo->GetAlwaysAskBeforeHandling(&alwaysAsk);
>   }
>   if (alwaysAsk)
>   {

nit: keep braces consistent.  i know the file was already inconsistent
in this case, but if you can fix at least this part that'd be great :)


sr=darin with nits fixed.
Attachment #126870 - Flags: superreview?(darin) → superreview+

Comment 15

15 years ago
Comment on attachment 126900 [details] [diff] [review]
fix other implementers

sr=darin
Attachment #126900 - Flags: superreview?(darin) → superreview+
Status: NEW → ASSIGNED
checked in. Bug 211576 filed for Camino. Bug 211577 filed for the activex
control.   Bug 211578 filed for mfcembed. 

Cocoa and Powerplant don't seem to show a dialog anyway, so I didn't file a bug.
We don't have a photon component, and I don't know who works on the photon port,
so I didn't file a bug for that.

Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 222757 has been marked as a duplicate of this bug. ***
Bug 285976 filed for firefox.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.