1306347 - Investigate whether multipart documents and sniffed types still need special treatment

Status: NEW

(Firefox :: File Handling, defect, P3)

Description

[:Paolo Amadini]

There is a comment in the code that invokes the handler choice dialog, and it reminds that the dialog should always be shown for multipart documents and sniffed types. We should make sure these cases don't need a special treatment before removing the dialog itself.

Comment 1

[William Hsu [:whsu]]

Hi, Paolo,

It seems to me that it's a investigation.
I temporarily set the priority to P3. Please feel free to change the priority to P1/P2 if we're going to complete it in this or next release.
Thank you. :)

Comment 2

[Ben Tian (inactive)]

(In reply to :Paolo Amadini from comment #0)
> There is a comment in the code that invokes the handler choice dialog, and
> it reminds that the dialog should always be shown for multipart documents
> and sniffed types. We should make sure these cases don't need a special
> treatment before removing the dialog itself.

The comment Paolo refers to:
http://searchfox.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1717

And the corresponding code:
http://searchfox.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1761

Comment 3

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #2)
> The comment Paolo refers to:
> http://searchfox.org/mozilla-central/source/uriloader/exthandler/
> nsExternalHelperAppService.cpp#1717

Boris,

Some questions about the code comment you left (no bug # from git blame:( ):
- Can you brief 1) how 'opening a part of multipart document' breaks some websites and 2) which kind of websites would be broken?
- Is the reason of always showing handler choice dialog to leave the open-or-not choice to user instead?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Some questions about the code comment you left (no bug # from git blame:( ):

From http://52.25.115.98/viewvc/main/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp?revision=1.296&view=markup :

   Only force prompting for non-"can't handle" reasons if the default action is
   not "save".  Silently saving is always ok.  Bug 236541, r=biesi, sr=darin

I guess you could get there by looking at http://searchfox.org/mozilla-central/commit/9578d6b9607d2b31ac1749febdda5f0345243838 too.  Silly importers messing up commit messages...

Anyway, that bug is a bit of a cesspit with all the people complaining in it, sorry.

> Can you brief 1) how 'opening a part of multipart document' breaks some websites

https://bugzilla.mozilla.org/show_bug.cgi?id=236541#c40 has some discussion, but briefly there were sites that wanted to allow users to save stuff and some helper apps didn't have a way to save the file once it was opened in a helper app.  So the user would be stuck not being able to save a part of a multipart response.

> - Is the reason of always showing handler choice dialog to leave the open-or-not choice to user instead?

Yes.

Comment 5

[Alphan Chen [:alchen]]

Currently, we don't remove the dialog. So we don't need to change the behavior for handling these file types.

Updating the status about this bug in the previous discussion with UX:
We should keep the original design(always pop the dialog) to deal with multipart documents and sniffed types before finding a better way for them. These cases will be treated as special cases if we removed the dialog.