642996 - Hide the "Save Link" context menu for some or all links

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

From bug 637031 comment 3 - comment 5:

madhava:
> We should be smarter about the inclusion of "Save Link" which is
> often useless when the page links to an html doc (I think it's
> there to allow saving of PDFs, etc.)

mbrubeck:
> We don't know reliably whether a link will return an HTML file or a PDF
> file until we follow it.  We could guess based on the file extension
> (e.g., hide the option if the URL ends in ".html") but this might be
> incorrect some of the time.  Would that be better or worse than showing
> it all the time?

madhava:
> It seems to me that it almost always a not useful option, in a menu that we're
> trying to keep as short as possible. Most of the time, a link is a link to
> another page, in which case we just save the HTML which is not a feature I
> think we would ever prioritize (or even want, particularly). What is a file
> that a person _would_ want to save, sight unseen (i.e. vs. first seeing an
> image file and long-tapping it directly to save it)?  PDFs already end up in
> the download manager.

Comment 1

[Matt Brubeck (:mbrubeck)]

The one potentially-important use case I've seen is files without correct Content-type headers.  These may be displayed in the browser when they ought to be saved.  Some discussion of this problem here:

https://support.mozilla.com/en-US/questions/792670

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Another use-case (using PDF as the example) is saving a PDF that won't be auto-deleted. If you just tap a PDF link, we open it in a PDF viewer but we will delete the PDF file as soon as we can.

To keep the PDF locally as long as you want, you need to explicitly _save_ it.

Comment 3

[Matt Brubeck (:mbrubeck)]

(In reply to comment #2)
> If you just tap a PDF link, we open it in a PDF viewer but we
> will delete the PDF file as soon as we can.

Perhaps we should file a bug to change this in Fennec - it seems inconsistent with the way our download manager works.  (I thought we had already, but I can't find one.)  For desktop this is bug 191239.

Comment 4

[Matt Brubeck (:mbrubeck)]

Filed bug 643027 for not deleting files on exit.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: patch

I think we can do this safely now that bug 596370 is fixed, which removes the objection from comment 2.  The problem in comment 1 remains, but it's an edge case that happens only on sites that are broken in most or all browsers.

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Remove some more code (thanks to Wes for comments via IRC).

Comment 7

[Wesley Johnston (:wesj)]

Comment on attachment 554552 [details] [diff] [review]
patch v2

Review of attachment 554552 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Thanks

Comment 8

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/16e0d32eb59c

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/16e0d32eb59c

Comment 10

[Aaron Train [:aaronmt]]

Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110830 Firefox/9.0a1 Fennec/9.0a1