From 5d1ade2f4b7e25852836191a863dbd23c228358e Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Fri, 10 Jun 2011 11:19:16 +1200 Subject: [PATCH 01/43] Bug 536503 - Last downloaded-to directory should be remembered on a site-by-site basis. r=gavin --- toolkit/content/contentAreaUtils.js | 15 +- toolkit/mozapps/downloads/DownloadLastDir.jsm | 63 +++-- toolkit/mozapps/downloads/nsHelperAppDlg.js | 8 +- .../tests/unit/test_DownloadLastDir.js | 1 + .../tests/unit/test_DownloadLastDirWithCPS.js | 232 ++++++++++++++++++ .../mozapps/downloads/tests/unit/xpcshell.ini | 1 + 6 files changed, 300 insertions(+), 20 deletions(-) create mode 100644 toolkit/mozapps/downloads/tests/unit/test_DownloadLastDirWithCPS.js diff --git a/toolkit/content/contentAreaUtils.js b/toolkit/content/contentAreaUtils.js index a6bdcdf400f1..731f59464b1e 100644 --- a/toolkit/content/contentAreaUtils.js +++ b/toolkit/content/contentAreaUtils.js @@ -341,7 +341,10 @@ function internalSave(aURL, aDocument, aDefaultFileName, aContentDisposition, file: file }; - if (!getTargetFile(fpParams, aSkipPrompt)) + // Find a URI to use for determining last-downloaded-to directory + let relatedURI = aReferrer || sourceURI; + + if (!getTargetFile(fpParams, aSkipPrompt, relatedURI)) // If the method returned false this is because the user cancelled from // the save file picker dialog. return; @@ -551,10 +554,14 @@ function initFileInfo(aFI, aURL, aURLCharset, aDocument, * If false, don't save the file automatically to the user's * default download directory, even if the associated preference * is set, but ask for the target explicitly. + * @param aRelatedURI + * An nsIURI associated with the download. The last used + * directory of the picker is retrieved from/stored in the + * Content Pref Service using this URI. * @return true if the user confirmed a filename in the picker or the picker * was not displayed; false if they dismissed the picker. */ -function getTargetFile(aFpP, /* optional */ aSkipPrompt) +function getTargetFile(aFpP, /* optional */ aSkipPrompt, /* optional */ aRelatedURI) { if (typeof gDownloadLastDir != "object") Components.utils.import("resource://gre/modules/DownloadLastDir.jsm"); @@ -587,7 +594,7 @@ function getTargetFile(aFpP, /* optional */ aSkipPrompt) // file picker if it is still valid. Otherwise, keep the default of the // user's default downloads directory. If it doesn't exist, it will be // changed to the user's desktop later. - var lastDir = gDownloadLastDir.file; + var lastDir = gDownloadLastDir.getFile(aRelatedURI); if (lastDir.exists()) { dir = lastDir; dirExists = true; @@ -631,7 +638,7 @@ function getTargetFile(aFpP, /* optional */ aSkipPrompt) // Do not store the last save directory as a pref inside the private browsing mode var directory = fp.file.parent.QueryInterface(nsILocalFile); - gDownloadLastDir.file = directory; + gDownloadLastDir.setFile(aRelatedURI, directory); fp.file.leafName = validateFileName(fp.file.leafName); diff --git a/toolkit/mozapps/downloads/DownloadLastDir.jsm b/toolkit/mozapps/downloads/DownloadLastDir.jsm index 77bb6343d2f3..f4011865a93a 100644 --- a/toolkit/mozapps/downloads/DownloadLastDir.jsm +++ b/toolkit/mozapps/downloads/DownloadLastDir.jsm @@ -19,6 +19,7 @@ * the Initial Developer. All Rights Reserved. * * Contributor(s): + * Geoff Lankow * * Alternatively, the contents of this file may be used under the terms of * either the GNU General Public License Version 2 or later (the "GPL"), or @@ -57,13 +58,15 @@ const nsILocalFile = Components.interfaces.nsILocalFile; var EXPORTED_SYMBOLS = [ "gDownloadLastDir" ]; +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); +Components.utils.import("resource://gre/modules/Services.jsm"); +Components.utils.import("resource://gre/modules/Dict.jsm"); + let pbSvc = null; if (PBSVC_CID in Components.classes) { pbSvc = Components.classes[PBSVC_CID] .getService(Components.interfaces.nsIPrivateBrowsingService); } -let prefSvc = Components.classes["@mozilla.org/preferences-service;1"] - .getService(Components.interfaces.nsIPrefBranch); let observer = { QueryInterface: function (aIID) { @@ -78,13 +81,17 @@ let observer = { case "private-browsing": if (aData == "enter") gDownloadLastDirFile = readLastDirPref(); - else if (aData == "exit") + else if (aData == "exit") { gDownloadLastDirFile = null; + gDownloadLastDirStore = new Dict(); + } break; case "browser:purge-session-history": gDownloadLastDirFile = null; - if (prefSvc.prefHasUserValue(LAST_DIR_PREF)) - prefSvc.clearUserPref(LAST_DIR_PREF); + if (Services.prefs.prefHasUserValue(LAST_DIR_PREF)) + Services.prefs.clearUserPref(LAST_DIR_PREF); + gDownloadLastDirStore = new Dict(); + Services.contentPrefs.removePrefsByName(LAST_DIR_PREF); break; } } @@ -97,7 +104,7 @@ os.addObserver(observer, "browser:purge-session-history", true); function readLastDirPref() { try { - return prefSvc.getComplexValue(LAST_DIR_PREF, nsILocalFile); + return Services.prefs.getComplexValue(LAST_DIR_PREF, nsILocalFile); } catch (e) { return null; @@ -105,8 +112,28 @@ function readLastDirPref() { } let gDownloadLastDirFile = readLastDirPref(); +let gDownloadLastDirStore = new Dict(); let gDownloadLastDir = { - get file() { + // compat shims + get file() { return this.getFile(); }, + set file(val) { this.setFile(null, val); }, + getFile: function (aURI) { + if (aURI) { + let lastDir; + if (pbSvc && pbSvc.privateBrowsingEnabled) { + let group = Services.contentPrefs.grouper.group(aURI); + lastDir = gDownloadLastDirStore.get(group, null); + } + if (!lastDir) { + lastDir = Services.contentPrefs.getPref(aURI, LAST_DIR_PREF); + } + if (lastDir) { + var lastDirFile = Components.classes["@mozilla.org/file/local;1"] + .createInstance(Components.interfaces.nsILocalFile); + lastDirFile.initWithPath(lastDir); + return lastDirFile; + } + } if (gDownloadLastDirFile && !gDownloadLastDirFile.exists()) gDownloadLastDirFile = null; @@ -115,17 +142,25 @@ let gDownloadLastDir = { else return readLastDirPref(); }, - set file(val) { + setFile: function (aURI, aFile) { + if (aURI) { + if (pbSvc && pbSvc.privateBrowsingEnabled) { + let group = Services.contentPrefs.grouper.group(aURI); + gDownloadLastDirStore.set(group, aFile.path); + } else { + Services.contentPrefs.setPref(aURI, LAST_DIR_PREF, aFile.path); + } + } if (pbSvc && pbSvc.privateBrowsingEnabled) { - if (val instanceof Components.interfaces.nsIFile) - gDownloadLastDirFile = val.clone(); + if (aFile instanceof Components.interfaces.nsIFile) + gDownloadLastDirFile = aFile.clone(); else gDownloadLastDirFile = null; } else { - if (val instanceof Components.interfaces.nsIFile) - prefSvc.setComplexValue(LAST_DIR_PREF, nsILocalFile, val); - else if (prefSvc.prefHasUserValue(LAST_DIR_PREF)) - prefSvc.clearUserPref(LAST_DIR_PREF); + if (aFile instanceof Components.interfaces.nsIFile) + Services.prefs.setComplexValue(LAST_DIR_PREF, nsILocalFile, aFile); + else if (Services.prefs.prefHasUserValue(LAST_DIR_PREF)) + Services.prefs.clearUserPref(LAST_DIR_PREF); } } }; diff --git a/toolkit/mozapps/downloads/nsHelperAppDlg.js b/toolkit/mozapps/downloads/nsHelperAppDlg.js index 58664eb6b18e..7c12bd94b183 100644 --- a/toolkit/mozapps/downloads/nsHelperAppDlg.js +++ b/toolkit/mozapps/downloads/nsHelperAppDlg.js @@ -312,9 +312,13 @@ nsUnknownContentTypeDialog.prototype = { .getService(Components.interfaces.nsIDownloadManager); picker.displayDirectory = dnldMgr.userDownloadsDirectory; + var relatedURI = null; + if (aContext.document) + relatedURI = aContext.document.documentURIObject; + // The last directory preference may not exist, which will throw. try { - var lastDir = gDownloadLastDir.file; + var lastDir = gDownloadLastDir.getFile(relatedURI); if (isUsableDirectory(lastDir)) picker.displayDirectory = lastDir; } @@ -343,7 +347,7 @@ nsUnknownContentTypeDialog.prototype = { var newDir = result.parent.QueryInterface(Components.interfaces.nsILocalFile); // Do not store the last save directory as a pref inside the private browsing mode - gDownloadLastDir.file = newDir; + gDownloadLastDir.setFile(relatedURI, newDir); result = this.validateLeafName(newDir, result.leafName, null); } diff --git a/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDir.js b/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDir.js index 07a31da6a9b0..cc67d93a2106 100644 --- a/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDir.js +++ b/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDir.js @@ -39,6 +39,7 @@ function run_test() let Cc = Components.classes; let Ci = Components.interfaces; let Cu = Components.utils; + do_get_profile(); Cu.import("resource://gre/modules/DownloadLastDir.jsm"); function clearHistory() { diff --git a/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDirWithCPS.js b/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDirWithCPS.js new file mode 100644 index 000000000000..f2259809ebcb --- /dev/null +++ b/toolkit/mozapps/downloads/tests/unit/test_DownloadLastDirWithCPS.js @@ -0,0 +1,232 @@ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is DownloadUtils Test Code. + * + * The Initial Developer of the Original Code is + * Geoff Lankow . + * Portions created by the Initial Developer are Copyright (C) 2009 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +const Cc = Components.classes; +const Ci = Components.interfaces; +const Cu = Components.utils; + +Cu.import("resource://gre/modules/DownloadLastDir.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); + +do_get_profile(); + +function run_test() { + function clearHistory() { + // simulate clearing the private data + Services.obs.notifyObservers(null, "browser:purge-session-history", ""); + } + + do_check_eq(typeof gDownloadLastDir, "object"); + do_check_eq(gDownloadLastDir.file, null); + + let tmpDir = Services.dirsvc.get("TmpD", Ci.nsILocalFile); + + let uri1 = Services.io.newURI("http://test1.com/", null, null); + let uri2 = Services.io.newURI("http://test2.com/", null, null); + let uri3 = Services.io.newURI("http://test3.com/", null, null); + let uri4 = Services.io.newURI("http://test4.com/", null, null); + + function newDir() { + let dir = tmpDir.clone(); + dir.append("testdir" + Math.floor(Math.random() * 10000)); + dir.QueryInterface(Ci.nsILocalFile); + dir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0700); + return dir; + } + + let dir1 = newDir(); + let dir2 = newDir(); + let dir3 = newDir(); + + try { + { // set up last dir + gDownloadLastDir.setFile(null, tmpDir); + do_check_eq(gDownloadLastDir.file.path, tmpDir.path); + do_check_neq(gDownloadLastDir.file, tmpDir); + } + + { // set uri1 to dir1, all should now return dir1 + // also check that a new object is returned + gDownloadLastDir.setFile(uri1, dir1); + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_neq(gDownloadLastDir.file, dir1); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); // set in CPS + do_check_neq(gDownloadLastDir.getFile(uri1), dir1); + do_check_eq(gDownloadLastDir.getFile(uri2).path, dir1.path); // fallback + do_check_neq(gDownloadLastDir.getFile(uri2), dir1); + do_check_eq(gDownloadLastDir.getFile(uri3).path, dir1.path); // fallback + do_check_neq(gDownloadLastDir.getFile(uri3), dir1); + do_check_eq(gDownloadLastDir.getFile(uri4).path, dir1.path); // fallback + do_check_neq(gDownloadLastDir.getFile(uri4), dir1); + } + + { // set uri2 to dir2, all except uri1 should now return dir2 + gDownloadLastDir.setFile(uri2, dir2); + do_check_eq(gDownloadLastDir.file.path, dir2.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri2).path, dir2.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri3).path, dir2.path); // fallback + do_check_eq(gDownloadLastDir.getFile(uri4).path, dir2.path); // fallback + } + + { // set uri3 to dir3, all except uri1 and uri2 should now return dir3 + gDownloadLastDir.setFile(uri3, dir3); + do_check_eq(gDownloadLastDir.file.path, dir3.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri2).path, dir2.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri3).path, dir3.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri4).path, dir3.path); // fallback + } + + { // set uri1 to dir2, all except uri3 should now return dir2 + gDownloadLastDir.setFile(uri1, dir2); + do_check_eq(gDownloadLastDir.file.path, dir2.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir2.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri2).path, dir2.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri3).path, dir3.path); // set in CPS + do_check_eq(gDownloadLastDir.getFile(uri4).path, dir2.path); // fallback + } + + { // check clearHistory removes all data + clearHistory(); + do_check_eq(gDownloadLastDir.file, null); + do_check_eq(Services.contentPrefs.hasPref(uri1, "lastDownloadDirectory"), false); + do_check_eq(gDownloadLastDir.getFile(uri1), null); + do_check_eq(gDownloadLastDir.getFile(uri2), null); + do_check_eq(gDownloadLastDir.getFile(uri3), null); + do_check_eq(gDownloadLastDir.getFile(uri4), null); + } + + let pb; + try { + pb = Cc["@mozilla.org/privatebrowsing;1"].getService(Ci.nsIPrivateBrowsingService); + } catch (e) { + print("PB service is not available, bail out"); + return; + } + + Services.prefs.setBoolPref("browser.privatebrowsing.keep_current_session", true); + + { // check data set outside PB mode is remembered + gDownloadLastDir.setFile(null, tmpDir); + pb.privateBrowsingEnabled = true; + do_check_eq(gDownloadLastDir.file.path, tmpDir.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, tmpDir.path); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file.path, tmpDir.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, tmpDir.path); + + clearHistory(); + } + + { // check data set using CPS outside PB mode is remembered + gDownloadLastDir.setFile(uri1, dir1); + pb.privateBrowsingEnabled = true; + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); + + clearHistory(); + } + + { // check data set inside PB mode is forgotten + pb.privateBrowsingEnabled = true; + gDownloadLastDir.setFile(null, tmpDir); + do_check_eq(gDownloadLastDir.file.path, tmpDir.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, tmpDir.path); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file, null); + do_check_eq(gDownloadLastDir.getFile(uri1), null); + + clearHistory(); + } + + { // check data set using CPS inside PB mode is forgotten + pb.privateBrowsingEnabled = true; + gDownloadLastDir.setFile(uri1, dir1); + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file, null); + do_check_eq(gDownloadLastDir.getFile(uri1), null); + + clearHistory(); + } + + { // check data set outside PB mode but changed inside is remembered correctly + gDownloadLastDir.setFile(uri1, dir1); + pb.privateBrowsingEnabled = true; + gDownloadLastDir.setFile(uri1, dir2); + do_check_eq(gDownloadLastDir.file.path, dir2.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir2.path); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); + + // check that the last dir store got cleared + pb.privateBrowsingEnabled = true; + do_check_eq(gDownloadLastDir.file.path, dir1.path); + do_check_eq(gDownloadLastDir.getFile(uri1).path, dir1.path); + + pb.privateBrowsingEnabled = false; + clearHistory(); + } + + { // check clearHistory inside PB mode clears data outside PB mode + pb.privateBrowsingEnabled = true; + gDownloadLastDir.setFile(uri1, dir2); + + clearHistory(); + do_check_eq(gDownloadLastDir.file, null); + do_check_eq(gDownloadLastDir.getFile(uri1), null); + + pb.privateBrowsingEnabled = false; + do_check_eq(gDownloadLastDir.file, null); + do_check_eq(gDownloadLastDir.getFile(uri1), null); + } + } finally { + dir1.remove(true); + dir2.remove(true); + dir3.remove(true); + } +} diff --git a/toolkit/mozapps/downloads/tests/unit/xpcshell.ini b/toolkit/mozapps/downloads/tests/unit/xpcshell.ini index 2299e87e811c..7c79798ffa85 100644 --- a/toolkit/mozapps/downloads/tests/unit/xpcshell.ini +++ b/toolkit/mozapps/downloads/tests/unit/xpcshell.ini @@ -3,6 +3,7 @@ head = tail = [test_DownloadLastDir.js] +[test_DownloadLastDirWithCPS.js] [test_DownloadPaths.js] [test_DownloadUtils.js] [test_lowMinutes.js] From 1ab0e726eb5c3b634d477081672f701927a59727 Mon Sep 17 00:00:00 2001 From: Steven Michaud Date: Sat, 11 Jun 2011 11:55:44 +0200 Subject: [PATCH 02/43] Bug 659817 - [10.7 SDK] Build error in PluginInterposeOSX.h due to Cursor type not being defined. r=bgirard --- dom/plugins/ipc/PluginInterposeOSX.h | 17 ++++++++++++ .../ipc/interpose/plugin_child_interpose.mm | 26 ++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/dom/plugins/ipc/PluginInterposeOSX.h b/dom/plugins/ipc/PluginInterposeOSX.h index 6f1ec9c3781c..6db92e9e47ee 100644 --- a/dom/plugins/ipc/PluginInterposeOSX.h +++ b/dom/plugins/ipc/PluginInterposeOSX.h @@ -42,6 +42,23 @@ class NSCursor; #import #endif +// The header file QuickdrawAPI.h is missing on OS X 10.7 and up (though the +// QuickDraw APIs defined in it are still present) -- so we need to supply the +// relevant parts of its contents here. It's likely that Apple will eventually +// remove the APIs themselves (probably in OS X 10.8), so we need to make them +// weak imports, and test for their presence before using them. +#if !defined(__QUICKDRAWAPI__) + +typedef short Bits16[16]; +struct Cursor { + Bits16 data; + Bits16 mask; + Point hotSpot; +}; +typedef struct Cursor Cursor; + +#endif /* __QUICKDRAWAPI__ */ + namespace mac_plugin_interposing { // Class used to serialize NSCursor objects over IPC between processes. diff --git a/dom/plugins/ipc/interpose/plugin_child_interpose.mm b/dom/plugins/ipc/interpose/plugin_child_interpose.mm index 519d6adfa57d..880436234e9c 100644 --- a/dom/plugins/ipc/interpose/plugin_child_interpose.mm +++ b/dom/plugins/ipc/interpose/plugin_child_interpose.mm @@ -64,6 +64,18 @@ #include #import +// The header file QuickdrawAPI.h is missing on OS X 10.7 and up (though the +// QuickDraw APIs defined in it are still present) -- so we need to supply the +// relevant parts of its contents here. It's likely that Apple will eventually +// remove the APIs themselves (probably in OS X 10.8), so we need to make them +// weak imports, and test for their presence before using them. +#if !defined(__QUICKDRAWAPI__) + +typedef struct Cursor; +extern "C" void SetCursor(const Cursor * crsr) __attribute__((weak_import)); + +#endif /* __QUICKDRAWAPI__ */ + BOOL (*OnSetThemeCursorPtr) (ThemeCursor) = NULL; BOOL (*OnSetCursorPtr) (const Cursor*) = NULL; BOOL (*OnHideCursorPtr) () = NULL; @@ -106,10 +118,12 @@ static OSStatus MacPluginChildSetThemeCursor(ThemeCursor cursor) static void MacPluginChildSetCursor(const Cursor* cursor) { - if (loadXULPtrs()) { - OnSetCursorPtr(cursor); + if (::SetCursor) { + if (loadXULPtrs()) { + OnSetCursorPtr(cursor); + } + ::SetCursor(cursor); } - ::SetCursor(cursor); } static CGError MacPluginChildCGDisplayHideCursor(CGDirectDisplayID display) @@ -142,9 +156,13 @@ struct interpose_substitution { __attribute__((used)) static const interpose_substitution substitutions[] __attribute__((section("__DATA, __interpose"))) = { INTERPOSE_FUNCTION(SetThemeCursor), - INTERPOSE_FUNCTION(SetCursor), INTERPOSE_FUNCTION(CGDisplayHideCursor), INTERPOSE_FUNCTION(CGDisplayShowCursor), + // SetCursor() and other QuickDraw APIs will probably be removed in OS X + // 10.8. But this will make 'SetCursor' NULL, which will just stop the OS + // from interposing it (tested using an INTERPOSE_FUNCTION_BROKEN macro + // that just sets the second address of each tuple to NULL). + INTERPOSE_FUNCTION(SetCursor), }; #endif // !__LP64__ From 22cc3fb3c7b2fa94d807d85f17a0f4f9e52602b8 Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Sat, 11 Jun 2011 15:01:15 +1200 Subject: [PATCH 03/43] Bug 659163 - Make menulists usable and useful in addon manager inline settings; r=dtownsend --- mobile/chrome/content/bindings/extensions.xml | 4 ++ .../mozapps/extensions/content/extensions.js | 44 +++++++------- .../browser_inlinesettings1/options.xul | 8 +-- .../test/browser/browser_inlinesettings.js | 59 ++++++++++++++++--- .../extensions/test/browser/options.xul | 3 + .../mozapps/extensions/extensions.css | 2 +- .../mozapps/extensions/extensions.css | 2 +- 7 files changed, 87 insertions(+), 35 deletions(-) diff --git a/mobile/chrome/content/bindings/extensions.xml b/mobile/chrome/content/bindings/extensions.xml index 29db8572a6b5..8ba48f46713b 100644 --- a/mobile/chrome/content/bindings/extensions.xml +++ b/mobile/chrome/content/bindings/extensions.xml @@ -138,6 +138,10 @@ let event = document.createEvent("Events"); event.initEvent("AddonOptionsLoad", true, false); this.dispatchEvent(event); + + // Also send a notification to match the behavior of desktop Firefox + let id = this.id.substring(17); // length of |urn:mozilla:item:| + Services.obs.notifyObservers(document, "addon-options-displayed", id); ]]> diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index 6dd1c6d2f6c0..abc4fe6ba04a 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -2821,6 +2821,22 @@ var gDetailView = { if (this._addon.optionsType != AddonManager.OPTIONS_TYPE_INLINE) return; + // This function removes and returns the text content of aNode without + // removing any child elements. Removing the text nodes ensures any XBL + // bindings apply properly. + function stripTextNodes(aNode) { + var text = ''; + for (var i = 0; i < aNode.childNodes.length; i++) { + if (aNode.childNodes[i].nodeType != document.ELEMENT_NODE) { + text += aNode.childNodes[i].textContent; + aNode.removeChild(aNode.childNodes[i--]); + } else { + text += stripTextNodes(aNode.childNodes[i]); + } + } + return text; + } + var rows = document.getElementById("detail-downloads").parentNode; var xhr = new XMLHttpRequest(); @@ -2830,39 +2846,21 @@ var gDetailView = { var xml = xhr.responseXML; var settings = xml.querySelectorAll(":root > setting"); - // This horrible piece of code fixes two problems. 1) The menulist binding doesn't apply - // correctly when it's moved from one document to another (bug 659163), which is solved - // by manually cloning the menulist. 2) Labels and controls aligned to the top of a row - // looks really bad, so the description is put on a new row to preserve alignment. for (var i = 0; i < settings.length; i++) { var setting = settings[i]; if (i == 0) setting.setAttribute("first-row", true); - // remove menulist controls for replacement later - var control = setting.firstElementChild; - if (setting.getAttribute("type") == "control" && control && control.localName == "menulist") { - setting.removeChild(control); - var consoleMessage = Cc["@mozilla.org/scripterror;1"]. - createInstance(Ci.nsIScriptError); - consoleMessage.init("Menulist is not available in the addons-manager yet, due to bug 659163", - this._addon.optionsURL, null, null, 0, Ci.nsIScriptError.warningFlag, null); - Services.console.logMessage(consoleMessage); - continue; - } - - // remove setting description, for replacement later - var desc = setting.textContent.trim(); - if (desc) - setting.textContent = ""; + // Remove setting description, for replacement later + var desc = stripTextNodes(setting).trim(); if (setting.hasAttribute("desc")) { - desc = setting.getAttribute("desc"); + desc = setting.getAttribute("desc").trim(); setting.removeAttribute("desc"); } rows.appendChild(setting); - // add a new row containing the description + // Add a new row containing the description if (desc) { var row = document.createElement("row"); var label = document.createElement("label"); @@ -2872,6 +2870,8 @@ var gDetailView = { rows.appendChild(row); } } + + Services.obs.notifyObservers(document, "addon-options-displayed", this._addon.id); }, getSelectedAddon: function() { diff --git a/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul b/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul index 4a4b45649032..ac23d3ecb3f4 100644 --- a/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul +++ b/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul @@ -5,11 +5,11 @@ - + - - - + + + diff --git a/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js b/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js index fc51b77ebe0a..005e12ddf463 100644 --- a/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js +++ b/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js @@ -8,6 +8,16 @@ var gManagerWindow; var gCategoryUtilities; var gProvider; +const SETTINGS_ROWS = 5; + +var observer = { + lastData: null, + observe: function(aSubject, aTopic, aData) { + if (aTopic == "addon-options-displayed") + this.lastData = aData; + } +}; + function installAddon(aCallback) { AddonManager.getInstallForURL(TESTROOT + "addons/browser_inlinesettings1.xpi", function(aInstall) { @@ -43,12 +53,16 @@ function test() { gManagerWindow = aWindow; gCategoryUtilities = new CategoryUtilities(gManagerWindow); + Services.obs.addObserver(observer, "addon-options-displayed", false); + run_next_test(); }); }); } function end_test() { + Services.obs.removeObserver(observer, "addon-options-displayed"); + close_manager(gManagerWindow, function() { AddonManager.getAddonByID("inlinesettings1@tests.mozilla.org", function(aAddon) { aAddon.uninstall(); @@ -102,9 +116,11 @@ add_test(function() { EventUtils.synthesizeMouseAtCenter(button, { clickCount: 1 }, gManagerWindow); wait_for_view_load(gManagerWindow, function() { + is(observer.lastData, "inlinesettings1@tests.mozilla.org", "Observer notification should have fired"); + var grid = gManagerWindow.document.getElementById("detail-grid"); var settings = grid.querySelectorAll("rows > setting"); - is(settings.length, 4, "Grid should have settings children"); + is(settings.length, SETTINGS_ROWS, "Grid should have settings children"); // Force bindings to apply settings[0].clientTop; @@ -151,10 +167,24 @@ add_test(function() { is(input.value, "bar", "Text box should have updated value"); is(Services.prefs.getCharPref("extensions.inlinesettings1.string"), "bar", "String pref should have been updated"); - var button = gManagerWindow.document.getElementById("detail-prefs-btn"); - is_element_hidden(button, "Preferences button should not be visible"); + var input = settings[4].firstElementChild; + is(input.value, "1", "Menulist should have initial value"); + input.focus(); + EventUtils.synthesizeKey("b", {}, gManagerWindow); + is(input.value, "2", "Menulist should have updated value"); + is(gManagerWindow._testValue, "2", "Menulist oncommand handler should've updated the test value"); - gCategoryUtilities.openType("extension", run_next_test); + setTimeout(function () { + EventUtils.synthesizeKey("c", {}, gManagerWindow); + is(input.value, "3", "Menulist should have updated value"); + is(gManagerWindow._testValue, "3", "Menulist oncommand handler should've updated the test value"); + delete gManagerWindow._testValue; + + var button = gManagerWindow.document.getElementById("detail-prefs-btn"); + is_element_hidden(button, "Preferences button should not be visible"); + + gCategoryUtilities.openType("extension", run_next_test); + }, 1200); // Timeout value from toolkit/content/tests/widgets/test_menulist_keynav.xul }); }); @@ -167,9 +197,11 @@ add_test(function() { EventUtils.synthesizeMouseAtCenter(button, { clickCount: 1 }, gManagerWindow); wait_for_view_load(gManagerWindow, function() { + is(observer.lastData, "inlinesettings2@tests.mozilla.org", "Observer notification should have fired"); + var grid = gManagerWindow.document.getElementById("detail-grid"); var settings = grid.querySelectorAll("rows > setting"); - is(settings.length, 2, "Grid should have settings children"); + is(settings.length, 3, "Grid should have settings children"); // Force bindings to apply settings[0].clientTop; @@ -192,6 +224,17 @@ add_test(function() { is(node.nodeName, "row", "Setting should be followed by a row node"); is(node.textContent, "Description Text Node", "Description should be in this row"); + node = settings[2]; + is(node.nodeName, "setting", "Should be a setting node"); + description = gManagerWindow.document.getAnonymousElementByAttribute(node, "class", "preferences-description"); + is(description.textContent.trim(), "", "Description node should be empty"); + var button = node.firstElementChild; + isnot(button, null, "There should be a button"); + + node = node.nextSibling; + is(node.nodeName, "row", "Setting should be followed by a row node"); + is(node.textContent.trim(), "This is a test, all this text should be visible", "Description should be in this row"); + var button = gManagerWindow.document.getElementById("detail-prefs-btn"); is_element_hidden(button, "Preferences button should not be visible"); @@ -208,6 +251,8 @@ add_test(function() { EventUtils.synthesizeMouseAtCenter(button, { clickCount: 1 }, gManagerWindow); wait_for_view_load(gManagerWindow, function() { + is(observer.lastData, "inlinesettings2@tests.mozilla.org", "Observer notification should not have fired"); + var grid = gManagerWindow.document.getElementById("detail-grid"); var settings = grid.querySelectorAll("rows > setting"); is(settings.length, 0, "Grid should not have settings children"); @@ -230,7 +275,7 @@ add_test(function() { wait_for_view_load(gManagerWindow, function() { var grid = gManagerWindow.document.getElementById("detail-grid"); var settings = grid.querySelectorAll("rows > setting"); - is(settings.length, 4, "Grid should have settings children"); + is(settings.length, SETTINGS_ROWS, "Grid should have settings children"); // disable var button = gManagerWindow.document.getElementById("detail-disable-btn"); @@ -259,7 +304,7 @@ add_test(function() { EventUtils.synthesizeMouseAtCenter(button, { clickCount: 1 }, gManagerWindow); settings = grid.querySelectorAll("rows > setting"); - is(settings.length, 4, "Grid should have settings children"); + is(settings.length, SETTINGS_ROWS, "Grid should have settings children"); gCategoryUtilities.openType("extension", run_next_test); }); diff --git a/toolkit/mozapps/extensions/test/browser/options.xul b/toolkit/mozapps/extensions/test/browser/options.xul index 46e9e5b1b3fc..07e9eb97c4d0 100644 --- a/toolkit/mozapps/extensions/test/browser/options.xul +++ b/toolkit/mozapps/extensions/test/browser/options.xul @@ -2,4 +2,7 @@ Description Text Node + + This is a test,