463772 - Make choice of skipping prompt for getTargetFile obvious

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Justin Wood (:Callek)]

Attachment: v1

This was brought about in my audit of the call-tree for "internalSave". Many functions were passing null (undefined) to it by never actually giving an argument up the ladder...

This turns those call sites into "false". I also fixed a potential future headache by having the context menu use saveFrame directly from gContextMenu instead of having two code-paths (that are synced).

Comment 1

[Mike Connor [:mconnor]]

its an optional param, this is fine.  what are you really trying to fix here?

Comment 2

[Justin Wood (:Callek)]

I just found the code easier to read and understand when we made this param explicit. If its best as WONTFIX in your opinion, no love lost ;-)

The one other thing I did do in this patch (which I can seperate out to a new bug if you feel it is worth it) is the contextMenu saveFrame to use gContextMenu instead.

Comment 3

[Shawn Wilsher :sdwilsh]

Comment on attachment 347032 [details] [diff] [review]
v1

So, I'd rather see us document the methods better and leave the false out (if it's documented as an optional argument that is).

Comment 4

[Justin Wood (:Callek)]

Attachment: v2 -- doc update, and fix context menu path

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 347684 [details] [diff] [review]
v2 -- doc update, and fix context menu path

I hate that version of javadoc comments, but I'm not going to ask you to change it.

r=sdwilsh

Comment 6

[Justin Wood (:Callek)]

Attachment: v2.1

address sdwilsh's super-nit (as discussed in irc). Mostly comment-only but needs approval as there is a minor unrelated code-change involved (to context menu). Definitely trivial.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347907 [details] [diff] [review]
v2.1

a191b2=beltzner

Comment 8

[Justin Wood (:Callek)]

$ hg push
pushing to ssh://hg.mozilla.org/mozilla-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 2 changes to 2 files

$ hg tip -v
changeset:   21713:6b19e8291f15
tag:         tip
user:        Justin Wood <[e-mail]>
date:        Sat Nov 15 18:19:10 2008 -0500
files:       browser/base/content/browser-context.inc toolkit/content/contentAreaUtils.js
description:
Bug 463772, Make choice of skipping prompt for getTargetFile obvious (comment changes)
Also include minor context-menu fix.
r=sdwilsh, a1.9.1b2=beltzner