Bug 422919 - Bookmark Import and Restore do not intelligently handle two different bookmark formats (r=mano, a=beltzner)
authordietrich@mozilla.com
Thu, 03 Apr 2008 13:51:54 -0700
changeset 13868 8c8e3697d5328c8db95fc542eab597aacc5ae442
parent 13867 e2a6aa8e2d3725308cbc2426162baf85529502e4
child 13869 1bb9078e19ab14deee9dfd0ab270b61c6d40fa7a
push id6
push userjorendorff@mozilla.com
push dateMon, 07 Apr 2008 22:38:53 +0000
treeherderautoland@7531959482c7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmano, beltzner
bugs422919
milestone1.9pre
Bug 422919 - Bookmark Import and Restore do not intelligently handle two different bookmark formats (r=mano, a=beltzner)
browser/app/profile/firefox.js
browser/components/migration/content/migration.xul
browser/components/places/content/places.js
browser/components/places/content/places.xul
browser/components/places/tests/unit/test_384370.js
browser/locales/en-US/chrome/browser/migration/migration.dtd
browser/locales/en-US/chrome/browser/places/places.dtd
browser/locales/en-US/chrome/browser/places/places.properties
toolkit/components/places/src/utils.js
toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js
toolkit/locales/en-US/chrome/places/places.properties
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -330,16 +330,22 @@ pref("browser.tabs.selectOwnerOnClose", 
 pref("browser.bookmarks.sort.direction", "descending");
 pref("browser.bookmarks.sort.resource", "rdf:http://home.netscape.com/NC-rdf#Name");
 
 // By default, do not export HTML at shutdown.
 // If true, at shutdown the bookmarks in your menu and toolbar will
 // be exported as HTML to the bookmarks.html file.
 pref("browser.bookmarks.autoExportHTML",          false);
 
+// The maximum number of daily bookmark backups to 
+// keep in {PROFILEDIR}/bookmarkbackups. Special values:
+// -1: unlimited
+//  0: no backups created (and deletes all existing backups)
+pref("browser.bookmarks.max_backups",             5);
+
 // Scripts & Windows prefs
 pref("dom.disable_open_during_load",              true);
 #ifdef DEBUG
 pref("javascript.options.showInConsole",          true);
 pref("general.warnOnAboutConfig",                 false);
 #else
 pref("javascript.options.showInConsole",          false);
 #endif
--- a/browser/components/migration/content/migration.xul
+++ b/browser/components/migration/content/migration.xul
@@ -101,17 +101,17 @@
       <radio id="opera"     label="&importFromOpera.label;"     accesskey="&importFromOpera.accesskey;"/>
 #endif
 #ifndef XP_UNIX
 #ifndef XP_WIN
       <radio id="seamonkey" label="&importFromSeamonkey.label;" accesskey="&importFromSeamonkey.accesskey;"/>
       <radio id="dogbert"   label="&importFromNetscape4.label;" accesskey="&importFromNetscape4.accesskey;"/>
 #endif
 #endif
-      <radio id="fromfile"  label="&importFromFile.label;"      accesskey="&importFromFile.accesskey;" hidden="true"/>
+      <radio id="fromfile"  label="&importFromHTMLFile.label;"  accesskey="&importFromHTMLFile.accesskey;" hidden="true"/>
       <radio id="nothing"   label="&importFromNothing.label;"   accesskey="&importFromNothing.accesskey;" hidden="true"/>
     </radiogroup>
     <label id="noSources" hidden="true">&noMigrationSources.label;</label>
   </wizardpage>
 
   <wizardpage id="selectProfile" pageid="selectProfile" label="&selectProfile.title;"
               next="importItems"
               onpageshow="return MigrationWizard.onSelectProfilePageShow();"
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@@ -293,28 +293,28 @@ var PlacesOrganizer = {
     var features = "modal,centerscreen,chrome,resizable=no";
 
     // The migrator window will set this to true when it closes, if the user
     // chose to migrate from a specific file.
     window.fromFile = false;
     openDialog("chrome://browser/content/migration/migration.xul",
                "migration", features, "bookmarks");
     if (window.fromFile)
-    this.importFromFile();
+      this.importFromFile();
   },
 
   /**
    * Open a file-picker and import the selected file into the bookmarks store
    */
   importFromFile: function PO_importFromFile() {
     var fp = Cc["@mozilla.org/filepicker;1"].
              createInstance(Ci.nsIFilePicker);
     fp.init(window, PlacesUIUtils.getString("SelectImport"),
             Ci.nsIFilePicker.modeOpen);
-    fp.appendFilters(Ci.nsIFilePicker.filterHTML | Ci.nsIFilePicker.filterAll);
+    fp.appendFilters(Ci.nsIFilePicker.filterHTML);
     if (fp.show() != Ci.nsIFilePicker.returnCancel) {
       if (fp.file) {
         var importer = Cc["@mozilla.org/browser/places/import-export-service;1"].
                        getService(Ci.nsIPlacesImportExportService);
         var file = fp.file.QueryInterface(Ci.nsILocalFile);
         importer.importHTMLFromFile(file, false);
       }
     }
@@ -348,34 +348,34 @@ var PlacesOrganizer = {
     while (restorePopup.childNodes.length > 1)
       restorePopup.removeChild(restorePopup.firstChild);
 
     // get list of files
     var fileList = [];
     var files = this.bookmarksBackupDir.directoryEntries;
     while (files.hasMoreElements()) {
       var f = files.getNext().QueryInterface(Ci.nsIFile);
-      if (!f.isHidden() && f.leafName.match(/^bookmarks-.+(html|json)?$/))
+      if (!f.isHidden() && f.leafName.match(/^bookmarks-.+json$/))
         fileList.push(f);
     }
 
     fileList.sort(function PO_fileList_compare(a, b) {
       return b.lastModifiedTime - a.lastModifiedTime;
     });
 
     if (fileList.length == 0)
       return;
 
     // populate menu
     for (var i = 0; i < fileList.length; i++) {
       var m = restorePopup.insertBefore
         (document.createElement("menuitem"),
          document.getElementById("restoreFromFile"));
       var dateStr = fileList[i].leafName.replace("bookmarks-", "").
-        replace(/\.(html|json)$/, "");
+        replace(/\.json$/, "");
       if (!dateStr.length)
         dateStr = fileList[i].leafName;
       m.setAttribute("label", dateStr);
       m.setAttribute("value", fileList[i].leafName);
       m.setAttribute("oncommand",
                      "PlacesOrganizer.onRestoreMenuItemClick(this);");
     }
     restorePopup.insertBefore(document.createElement("menuseparator"),
@@ -399,91 +399,109 @@ var PlacesOrganizer = {
   /**
    * Called when 'Choose File...' is selected from the restore menu.
    * Prompts for a file and restores bookmarks to those in the file.
    */
   onRestoreBookmarksFromFile: function PO_onRestoreBookmarksFromFile() {
     var fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
     fp.init(window, PlacesUIUtils.getString("bookmarksRestoreTitle"),
             Ci.nsIFilePicker.modeOpen);
+    fp.appendFilter(PlacesUIUtils.getString("bookmarksRestoreFilterName"),
+                    PlacesUIUtils.getString("bookmarksRestoreFilterExtension"));
 
     var dirSvc = Cc["@mozilla.org/file/directory_service;1"].
                  getService(Ci.nsIProperties);
     var backupsDir = dirSvc.get("Desk", Ci.nsILocalFile);
     fp.displayDirectory = backupsDir;
 
     if (fp.show() != Ci.nsIFilePicker.returnCancel)
-      PlacesUtils.restoreBookmarksFromFile(fp.file);
+      this.restoreBookmarksFromFile(fp.file);
   },
 
   /**
-   * Restores bookmarks from an HTML or JSON file.
+   * Restores bookmarks from a JSON file.
    */
   restoreBookmarksFromFile: function PO_restoreBookmarksFromFile(aFile) {
+    // check file extension
+    if (!aFile.leafName.match(/\.json$/)) {
+      this._showErrorAlert(PlacesUIUtils.getString("bookmarksRestoreFormatError"));
+      return;
+    }
+
+    // confirm ok to delete existing bookmarks
     var prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"].
                   getService(Ci.nsIPromptService);
     if (!prompts.confirm(null,
                          PlacesUIUtils.getString("bookmarksRestoreAlertTitle"),
                          PlacesUIUtils.getString("bookmarksRestoreAlert")))
       return;
 
-    if (aFile.leafName.match("\.json$")) {
-      // restore a JSON backup
+    try {
       PlacesUtils.restoreBookmarksFromJSONFile(aFile);
     }
-    else {
-      var importer = Cc["@mozilla.org/browser/places/import-export-service;1"].
-                     getService(Ci.nsIPlacesImportExportService);
-      importer.importHTMLFromFile(aFile, true /* overwrite existing */);
+    catch(ex) {
+      this._showErrorAlert(PlacesUIUtils.getString("bookmarksRestoreParseError"));
     }
   },
 
+  _showErrorAlert: function PO__showErrorAlert(aMsg) {
+    var brandShortName = document.getElementById("brandStrings").
+                                  getString("brandShortName");
+
+    Cc["@mozilla.org/embedcomp/prompt-service;1"].
+      getService(Ci.nsIPromptService).
+      alert(window, brandShortName, aMsg);
+  },
+
   /**
    * Backup bookmarks to desktop, auto-generate a filename with a date.
    * The file is a JSON serialization of bookmarks, tags and any annotations
    * of those items.
    */
   backupBookmarks: function PO_backupBookmarks() {
     var fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
     fp.init(window, PlacesUIUtils.getString("bookmarksBackupTitle"),
             Ci.nsIFilePicker.modeSave);
+    fp.appendFilter(PlacesUIUtils.getString("bookmarksRestoreFilterName"),
+                    PlacesUIUtils.getString("bookmarksRestoreFilterExtension"));
 
     var dirSvc = Cc["@mozilla.org/file/directory_service;1"].
                  getService(Ci.nsIProperties);
     var backupsDir = dirSvc.get("Desk", Ci.nsILocalFile);
     fp.displayDirectory = backupsDir;
 
     // Use YYYY-MM-DD (ISO 8601) as it doesn't contain illegal characters
     // and makes the alphabetical order of multiple backup files more useful.
     var date = (new Date).toLocaleFormat("%Y-%m-%d");
-    fp.defaultString = PlacesUtils.getFormattedString("bookmarksBackupFilename",
+    fp.defaultString = PlacesUIUtils.getFormattedString("bookmarksBackupFilenameJSON",
                                                         [date]);
 
     if (fp.show() != Ci.nsIFilePicker.returnCancel) {
       PlacesUtils.backupBookmarksToFile(fp.file);
 
       // copy new backup to /backups dir (bug 424389)
       var latestBackup = PlacesUtils.getMostRecentBackup();
       if (latestBackup != fp.file) {
+        latestBackup.remove(false);
         var date = new Date().toLocaleFormat("%Y-%m-%d");
         var name = PlacesUtils.getFormattedString("bookmarksArchiveFilename",
                                                   [date]);
         fp.file.copyTo(this.bookmarksBackupDir, name);
       }
     }
   },
 
   get bookmarksBackupDir() {
     delete this.bookmarksBackupDir;
     var dirSvc = Cc["@mozilla.org/file/directory_service;1"].
                  getService(Ci.nsIProperties);
     var bookmarksBackupDir = dirSvc.get("ProfD", Ci.nsIFile);
     bookmarksBackupDir.append("bookmarkbackups");
     if (!bookmarksBackupDir.exists())
-      bookmarksBackupDir.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0600);
+      bookmarksBackupDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0700);
     return this.bookmarksBackupDir = bookmarksBackupDir;
   },
 
   _paneDisabled: false,
   _setDetailsFieldsDisabledState:
   function PO__setDetailsFieldsDisabledState(aDisabled) {
     if (aDisabled) {
       document.getElementById("paneElementsBroadcaster")
--- a/browser/components/places/content/places.xul
+++ b/browser/components/places/content/places.xul
@@ -81,16 +81,21 @@
 
   <script type="application/x-javascript"
           src="chrome://browser/content/places/places.js"/>
   <script type="application/javascript"
           src="chrome://browser/content/utilityOverlay.js"/>
   <script type="application/javascript"
           src="chrome://browser/content/places/editBookmarkOverlay.js"/>
 
+  <stringbundleset id="placesStringSet">
+    <stringbundle id="brandStrings" src="chrome://branding/locale/brand.properties"/>
+  </stringbundleset>
+
+
 #ifdef XP_MACOSX
 #include ../../../base/content/browserMountPoints.inc
 #else
   <commandset id="editMenuCommands"/>
   <commandset id="placesCommands"/>
 #endif
 
   <commandset id="organizerCommandSet">
@@ -341,38 +346,38 @@
         </toolbarbutton>
         <toolbarbutton type="menu" class="tabbable"
 #else
         </menu>
         <menu accesskey="&maintenance.accesskey;" class="menu-iconic"
 #endif
               id="maintenanceButton" label="&maintenance.label;">
           <menupopup id="maintenanceButtonPopup">
-            <menuitem id="fileImport"
-                      command="OrganizerCommand_import"
-                      label="&cmd.import.label;"
-                      accesskey="&cmd.import.accesskey;"/>
-            <menuitem id="fileExport"
-                      command="OrganizerCommand_export"
-                      label="&cmd.export.label;"
-                      accesskey="&cmd.export.accesskey;"/>
-            <menuseparator/>
             <menuitem id="backupBookmarks"
                       command="OrganizerCommand_backup"
                       label="&cmd.backup.label;"
                       accesskey="&cmd.backup.accesskey;"/>
-            <menu id="fileRestoreMenu" label="&cmd.restore.label;"
+            <menu id="fileRestoreMenu" label="&cmd.restore2.label;"
                       accesskey="&cmd.restore.accesskey;">
               <menupopup id="fileRestorePopup" onpopupshowing="PlacesOrganizer.populateRestoreMenu();">
                 <menuitem id="restoreFromFile"
                           command="OrganizerCommand_restoreFromFile"
                           label="&cmd.restoreFromFile.label;"
                           accesskey="&cmd.restoreFromFile.accesskey;"/>
               </menupopup>
             </menu>
+            <menuseparator/>
+            <menuitem id="fileImport"
+                      command="OrganizerCommand_import"
+                      label="&cmd.importHTML.label;"
+                      accesskey="&cmd.importHTML.accesskey;"/>
+            <menuitem id="fileExport"
+                      command="OrganizerCommand_export"
+                      label="&cmd.exportHTML.label;"
+                      accesskey="&cmd.exportHTML.accesskey;"/>
           </menupopup>
 #ifdef XP_MACOSX
         </toolbarbutton>
 #else
         </menu>
       </menubar>
 #endif
 
--- a/browser/components/places/tests/unit/test_384370.js
+++ b/browser/components/places/tests/unit/test_384370.js
@@ -94,17 +94,17 @@ function run_test() {
   // 2. empty bookmarks db
   // 3. import bookmarks.exported.json
   // 4. run the test-suite
   try {
     PlacesUtils.backupBookmarksToFile(jsonFile);
   } catch(ex) { do_throw("couldn't export to file: " + ex); }
   LOG("exported json"); 
   try {
-    PlacesUtils.restoreBookmarksFromFile(jsonFile);
+    PlacesUtils.restoreBookmarksFromJSONFile(jsonFile);
   } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
   LOG("imported json"); 
   validate();
   LOG("validated import"); 
 }
 
 var tagData = [
   { uri: uri("http://slint.us"), tags: ["indie", "kentucky", "music"] },
--- a/browser/locales/en-US/chrome/browser/migration/migration.dtd
+++ b/browser/locales/en-US/chrome/browser/migration/migration.dtd
@@ -26,18 +26,18 @@
 <!ENTITY importFromICab.label           "iCab">
 <!ENTITY importFromICab.accesskey       "i">
 <!ENTITY importFromKonqueror.label      "Konqueror">
 <!ENTITY importFromKonqueror.accesskey  "K">
 <!ENTITY importFromEpiphany.label       "Epiphany">
 <!ENTITY importFromEpiphany.accesskey   "E">
 <!ENTITY importFromGaleon.label         "Galeon">
 <!ENTITY importFromGaleon.accesskey     "G">
-<!ENTITY importFromFile.label           "From File">
-<!ENTITY importFromFile.accesskey       "F">
+<!ENTITY importFromHTMLFile.label       "From an HTML File">
+<!ENTITY importFromHTMLFile.accesskey   "F">
 
 <!ENTITY noMigrationSources.label       "No programs that contain bookmarks, history or password data could be found.">
 
 <!ENTITY importSource.title             "Import Settings and Data">
 <!ENTITY importItems.title              "Items to Import">
 <!ENTITY importItems.label              "Select which items to import:">
 
 <!ENTITY migrating.title                "Importing…">
--- a/browser/locales/en-US/chrome/browser/places/places.dtd
+++ b/browser/locales/en-US/chrome/browser/places/places.dtd
@@ -32,24 +32,24 @@
 <!ENTITY help.advancedTips.label        "Advanced Tips &amp; Tricks">
 <!ENTITY help.advancedTips.accesskey    "T">
 
 <!ENTITY cmd.findInBookmarks.label      "Find in Bookmarks…">
 <!ENTITY cmd.findInBookmarks.accesskey  "F">
 <!ENTITY cmd.findCurrent.label          "Find in Current Collection…">
 <!ENTITY cmd.findCurrent.accesskey      "i">
 
-<!ENTITY cmd.export.label      "Export…">
-<!ENTITY cmd.export.accesskey  "E">
-<!ENTITY cmd.import.label      "Import…">
-<!ENTITY cmd.import.accesskey  "I">
+<!ENTITY cmd.exportHTML.label           "Export HTML…">
+<!ENTITY cmd.exportHTML.accesskey       "E">
+<!ENTITY cmd.importHTML.label           "Import HTML…">
+<!ENTITY cmd.importHTML.accesskey       "I">
 
 <!ENTITY cmd.backup.label               "Backup…">
 <!ENTITY cmd.backup.accesskey           "B">
-<!ENTITY cmd.restore.label              "Restore…">
+<!ENTITY cmd.restore2.label             "Restore">
 <!ENTITY cmd.restore.accesskey          "R">
 <!ENTITY cmd.restoreFromFile.label      "Choose File…">
 <!ENTITY cmd.restoreFromFile.accesskey  "C">
 
 <!ENTITY cmd.select_all.label "Select All">
 <!ENTITY cmd.select_all.accesskey "A">
 <!ENTITY cmd.select_all.key "a">
 
--- a/browser/locales/en-US/chrome/browser/places/places.properties
+++ b/browser/locales/en-US/chrome/browser/places/places.properties
@@ -18,16 +18,21 @@ bookmarksMenuEmptyFolder=(Empty)
 bookmarksBackupFilename=Bookmarks %S.html
 bookmarksBackupFilenameJSON=Bookmarks %S.json
 bookmarksBackupTitle=Bookmarks backup filename
 
 bookmarksRestoreAlertTitle=Revert Bookmarks
 bookmarksRestoreAlert=This will replace all of your current bookmarks with the backup. Are you sure?
 bookmarksRestoreAlertTags=This will replace all of your current bookmarks and tags with the backup. Are you sure?
 bookmarksRestoreTitle=Select a bookmarks backup
+bookmarksRestoreFilterName=JSON
+bookmarksRestoreFilterExtension=*.json
+
+bookmarksRestoreFormatError=Unsupported file type.
+bookmarksRestoreParseError=Unable to process the backup file.
 
 headerTextPrefix1=Showing 
 headerTextPrefix2=Search Results for 
 headerTextPrefix3=Advanced Search
 
 lessCriteria.label=-
 moreCriteria.label=+
 
--- a/toolkit/components/places/src/utils.js
+++ b/toolkit/components/places/src/utils.js
@@ -948,57 +948,16 @@ var PlacesUtils = {
     return urls;
   },
 
   /**
    * Restores bookmarks/tags from a JSON file.
    * WARNING: This method *removes* any bookmarks in the collection before
    * restoring from the file.
    */
-  restoreBookmarksFromFile: function PU_restoreBookmarksFromFile(aFile) {
-    var errorStr = null;
-
-    var ioSvc = Cc["@mozilla.org/network/io-service;1"].
-                 getService(Ci.nsIIOService);
-    var fileURL = ioSvc.newFileURI(aFile).QueryInterface(Ci.nsIURL);
-    var fileExtension = fileURL.fileExtension.toLowerCase();
-
-    if (fileExtension == "json") {
-      try {
-        this.restoreBookmarksFromJSONFile(aFile);
-      } catch(ex) {
-        errorStr = this.getString("restoreParseError");
-      }
-    }
-    else {
-      errorStr = this.getString("restoreFormatError");
-    }
-
-    if (errorStr) {
-      const BRANDING_BUNDLE_URI = "chrome://branding/locale/brand.properties";
-      var brandShortName = Cc["@mozilla.org/intl/stringbundle;1"].
-                           getService(Ci.nsIStringBundleService).
-                           createBundle(BRANDING_BUNDLE_URI).
-                           GetStringFromName("brandShortName");
-
-      var wm = Cc["@mozilla.org/appshell/window-mediator;1"].
-               getService(Ci.nsIWindowMediator);
-      var win = wm.getMostRecentWindow(null);
-
-      Cc["@mozilla.org/embedcomp/prompt-service;1"].
-        getService(Ci.nsIPromptService).
-        alert(win, brandShortName, errorStr);
-    }
-  },
-
-  /**
-   * Restores bookmarks/tags from a JSON file.
-   * WARNING: This method *removes* any bookmarks in the collection before
-   * restoring from the file.
-   */
   restoreBookmarksFromJSONFile:
   function PU_restoreBookmarksFromJSONFile(aFile) {
     // open file stream
     var stream = Cc["@mozilla.org/network/file-input-stream;1"].
                  createInstance(Ci.nsIFileInputStream);
     stream.init(aFile, 0x01, 0, 0);
     var converted = Cc["@mozilla.org/intl/converter-input-stream;1"].
                     createInstance(Ci.nsIConverterInputStream);
--- a/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js
+++ b/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js
@@ -113,17 +113,17 @@ function run_test() {
 
   // clean
   tests.forEach(function(aTest) {
     aTest.clean();
   });
 
   // restore json file
   try {
-    PlacesUtils.restoreBookmarksFromFile(jsonFile);
+    PlacesUtils.restoreBookmarksFromJSONFile(jsonFile);
   } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
 
   // validate
   tests.forEach(function(aTest) {
     aTest.validate();
   });
 
   // clean up
--- a/toolkit/locales/en-US/chrome/places/places.properties
+++ b/toolkit/locales/en-US/chrome/places/places.properties
@@ -8,19 +8,14 @@ bookmarksLivemarkFailed=Live Bookmark fe
 
 finduri-AgeInDays-is-0=Today
 finduri-AgeInDays-is-1=Yesterday
 finduri-AgeInDays-is=%S days ago
 finduri-AgeInDays-isgreater=Older than %S days
 
 localhost=(local files)
 
-# LOCALIZATION NOTE (bookmarksBackupFilename & bookmarksArchiveFilename):
+# LOCALIZATION NOTE (bookmarksArchiveFilename):
 # %S will be replaced by the current date in ISO 8601 format, YYYY-MM-DD.
 # The resulting string will be suggested as a filename, so make sure that you're
 # only using characters legal for file names. Consider falling back to the
 # en-US value if you have to use non-ascii characters.
-bookmarksBackupFilename=Bookmarks %S.json
-bookmarksBackupTitle=Bookmarks backup filename
 bookmarksArchiveFilename=bookmarks-%S.json
-
-restoreFormatError=Unsupported file type.
-restoreParseError=Unable to process the backup file.