Bug 1452068 - Move MAX_TAG_LENGTH to the bookmarking API. r=Standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 24 Jul 2018 18:45:13 +0000
changeset 428121 7cec8c9008b7525f42c05853355ec6def7e76ef0
parent 428120 a2c7cdc17db17c824bbab21d0c317e129cfe2bb6
child 428122 163ad940f0789bcf8735dce83dad5903ac841db5
push id66835
push usermak77@bonardo.net
push dateWed, 25 Jul 2018 12:51:02 +0000
treeherderautoland@7cec8c9008b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1452068
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1452068 - Move MAX_TAG_LENGTH to the bookmarking API. r=Standard8 The tagging API is moving to the bookmarking API, this is one step Differential Revision: https://phabricator.services.mozilla.com/D2315
toolkit/components/places/BookmarkHTMLUtils.jsm
toolkit/components/places/BookmarkJSONUtils.jsm
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/nsITaggingService.idl
toolkit/components/places/nsTaggingService.js
toolkit/components/places/tests/unit/test_tagging.js
--- a/toolkit/components/places/BookmarkHTMLUtils.jsm
+++ b/toolkit/components/places/BookmarkHTMLUtils.jsm
@@ -554,17 +554,17 @@ BookmarkImporter.prototype = {
       // This is a livemark, we've done all we need to do here, so finish early.
       frame.folder.children.push(bookmark);
       frame.previousItem = bookmark;
       return;
     }
 
     if (tags) {
       bookmark.tags = tags.split(",").filter(aTag => aTag.length > 0 &&
-        aTag.length <= Ci.nsITaggingService.MAX_TAG_LENGTH);
+        aTag.length <= PlacesUtils.bookmarks.MAX_TAG_LENGTH);
 
       // If we end up with none, then delete the property completely.
       if (!bookmark.tags.length) {
         delete bookmark.tags;
       }
     }
 
     if (lastCharset) {
--- a/toolkit/components/places/BookmarkJSONUtils.jsm
+++ b/toolkit/components/places/BookmarkJSONUtils.jsm
@@ -433,17 +433,17 @@ function translateTreeTypes(node) {
     } else {
       delete node.lastModified;
     }
   }
 
   if (node.tags) {
      // Separate any tags into an array, and ignore any that are too long.
     node.tags = node.tags.split(",").filter(aTag =>
-      aTag.length > 0 && aTag.length <= Ci.nsITaggingService.MAX_TAG_LENGTH);
+      aTag.length > 0 && aTag.length <= PlacesUtils.bookmarks.MAX_TAG_LENGTH);
 
     // If we end up with none, then delete the property completely.
     if (!node.tags.length) {
       delete node.tags;
     }
   }
 
   // Sometimes postData can be null, so delete it to make the validators happy.
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -119,16 +119,22 @@ var Bookmarks = Object.freeze({
 
   /**
    * Default index used to append a bookmark-item at the end of a folder.
    * This should stay consistent with nsINavBookmarksService.idl
    */
   DEFAULT_INDEX: -1,
 
   /**
+   * Maximum length of a tag.
+   * Any tag above this length is rejected.
+   */
+  MAX_TAG_LENGTH: 100,
+
+  /**
    * Bookmark change source constants, passed as optional properties and
    * forwarded to observers. See nsINavBookmarksService.idl for an explanation.
    */
   SOURCES: {
     DEFAULT: Ci.nsINavBookmarksService.SOURCE_DEFAULT,
     SYNC: Ci.nsINavBookmarksService.SOURCE_SYNC,
     IMPORT: Ci.nsINavBookmarksService.SOURCE_IMPORT,
     SYNC_REPARENT_REMOVED_FOLDER_CHILDREN: Ci.nsINavBookmarksService.SOURCE_SYNC_REPARENT_REMOVED_FOLDER_CHILDREN,
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -262,27 +262,27 @@ const SYNC_BOOKMARK_VALIDATORS = Object.
                                 (PlacesSyncUtils.bookmarks.ROOTS.includes(v) || PlacesUtils.isValidGuid(v)))),
   parentRecordId: v => SYNC_BOOKMARK_VALIDATORS.recordId(v),
   // Sync uses kinds instead of types, which distinguish between livemarks and
   // queries.
   kind: simpleValidateFunc(v => typeof v == "string" &&
                                 Object.values(PlacesSyncUtils.bookmarks.KINDS).includes(v)),
   query: simpleValidateFunc(v => v === null || (typeof v == "string" && v)),
   folder: simpleValidateFunc(v => typeof v == "string" && v &&
-                                  v.length <= Ci.nsITaggingService.MAX_TAG_LENGTH),
+                                  v.length <= PlacesUtils.bookmarks.MAX_TAG_LENGTH),
   tags: v => {
     if (v === null) {
       return [];
     }
     if (!Array.isArray(v)) {
       throw new Error("Invalid tag array");
     }
     for (let tag of v) {
       if (typeof tag != "string" || !tag ||
-          tag.length > Ci.nsITaggingService.MAX_TAG_LENGTH) {
+          tag.length > PlacesUtils.bookmarks.MAX_TAG_LENGTH) {
         throw new Error(`Invalid tag: ${tag}`);
       }
     }
     return v;
   },
   keyword: simpleValidateFunc(v => v === null || typeof v == "string"),
   dateAdded: simpleValidateFunc(v => typeof v === "number"
     && v > PlacesSyncUtils.bookmarks.EARLIEST_BOOKMARK_TIMESTAMP),
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -2795,17 +2795,17 @@ function validateKeyword(rawKeyword) {
   return keyword ? keyword.toLowerCase() : null;
 }
 
 function validateTag(rawTag) {
   if (typeof rawTag != "string") {
     return null;
   }
   let tag = rawTag.trim();
-  if (!tag || tag.length > Ci.nsITaggingService.MAX_TAG_LENGTH) {
+  if (!tag || tag.length > PlacesUtils.bookmarks.MAX_TAG_LENGTH) {
     // Drop empty and oversized tags.
     return null;
   }
   return tag;
 }
 
 // Recursively inflates a bookmark tree from a pseudo-tree that maps
 // parents to children.
--- a/toolkit/components/places/nsITaggingService.idl
+++ b/toolkit/components/places/nsITaggingService.idl
@@ -6,23 +6,16 @@
 #include "nsISupports.idl"
 
 interface nsIURI;
 interface nsIVariant;
 
 [scriptable, uuid(9759bd0e-78e2-4421-9ed1-c676e1af3513)]
 interface nsITaggingService : nsISupports
 {
-
-  /**
-   * Defines the maximal length of a tag. Related to the bug 407821 
-   * (https://bugzilla.mozilla.org/show_bug.cgi?id=407821) 
-   */
-  const unsigned long MAX_TAG_LENGTH = 100;
-
   /**
    * Tags a URL with the given set of tags. Current tags set for the URL
    * persist. Tags in aTags which are already set for the given URL are
    * ignored.
    *
    * @param aURI
    *        the URL to tag.
    * @param aTags
--- a/toolkit/components/places/nsTaggingService.js
+++ b/toolkit/components/places/nsTaggingService.js
@@ -109,17 +109,17 @@ TaggingService.prototype = {
       let tag = {};
       if (typeof(idOrName) == "number" && this._tagFolders[idOrName]) {
         // This is a tag folder id.
         tag.id = idOrName;
         // We can't know the name at this point, since a previous tag could
         // want to change it.
         tag.__defineGetter__("name", () => this._tagFolders[tag.id]);
       } else if (typeof(idOrName) == "string" && idOrName.length > 0 &&
-               idOrName.length <= Ci.nsITaggingService.MAX_TAG_LENGTH) {
+               idOrName.length <= PlacesUtils.bookmarks.MAX_TAG_LENGTH) {
         // This is a tag name.
         tag.name = trim ? idOrName.trim() : idOrName;
         // We can't know the id at this point, since a previous tag could
         // have created it.
         tag.__defineGetter__("id", () => this._getItemIdForTag(tag.name));
       } else {
         throw Components.Exception("Invalid tag value", Cr.NS_ERROR_INVALID_ARG);
       }
--- a/toolkit/components/places/tests/unit/test_tagging.js
+++ b/toolkit/components/places/tests/unit/test_tagging.js
@@ -152,27 +152,25 @@ function run_test() {
   }
   try {
     tagssvc.tagURI(uri1, [0, "test"]);
     do_throw("Passing a bad tags array should throw");
   } catch (ex) {
     Assert.equal(ex.name, "NS_ERROR_ILLEGAL_VALUE");
   }
 
-  // Tag name length should be limited to nsITaggingService.MAX_TAG_LENGTH (bug407821)
+  // Tag name length should be limited to PlacesUtils.bookmarks.MAX_TAG_LENGTH (bug407821)
   try {
-
     // generate a long tag name. i.e. looooo...oong_tag
-    var n = Ci.nsITaggingService.MAX_TAG_LENGTH;
+    var n = PlacesUtils.bookmarks.MAX_TAG_LENGTH;
     var someOos = new Array(n).join("o");
     var longTagName = "l" + someOos + "ng_tag";
 
     tagssvc.tagURI(uri1, ["short_tag", longTagName]);
     do_throw("Passing a bad tags array should throw");
-
   } catch (ex) {
     Assert.equal(ex.name, "NS_ERROR_ILLEGAL_VALUE");
   }
 
   // cleanup
   tagRoot.containerOpen = false;
 
   // Tagging service should trim tags (Bug967196)