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

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

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.....

Comment 1

[Boris Zbarsky [:bzbarsky]]

.

Comment 2

[Boris Zbarsky [:bzbarsky]]

*** Bug 186552 has been marked as a duplicate of this bug. ***

Comment 3

[Boris Zbarsky [:bzbarsky]]

*** Bug 209761 has been marked as a duplicate of this bug. ***

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

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".

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

ok. taking.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

implement what bz described before.

This will make the checkbox in the dialog not reflect the MIMEInfo value (for
content-disposition:attachment content)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 126870 [details] [diff] [review]
patch

r=bzbarsky.  you're on a roll, man!

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: fix other implementers

this is needed in addition to the other patch

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

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

this time, including the necessary camino change.

Comment 12

[Boris Zbarsky [: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.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

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

bz: ok, I'll file those bugs when I checkin this patch

Comment 14

[Darin Fisher]

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.

Comment 15

[Darin Fisher]

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

sr=darin

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 17

[Boris Zbarsky [:bzbarsky]]

*** Bug 222757 has been marked as a duplicate of this bug. ***

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Bug 285976 filed for firefox.