Closed Bug 463772 Opened 11 years ago Closed 11 years ago
Make choice of skipping prompt for get
Target File obvious
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).
its an optional param, this is fine. what are you really trying to fix here?
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 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).
Attachment #347684 - Flags: review?(sdwilsh) → review+
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
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 on attachment 347907 [details] [diff] [review] v2.1 a191b2=beltzner
Attachment #347907 - Flags: approval1.9.1b2? → approval1.9.1b2+
$ 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 <Callek@gmail.com> 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
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.