Bug 1486783 - Fix some undefined property references to description and previewImageURL in PlacesUtils.jsm and History.jsm. r=mak
authorMark Banner <standard8@mozilla.com>
Thu, 30 Aug 2018 13:50:14 +0000
changeset 434055 24ecf2822560
parent 434054 4a462d6e9c46
child 434056 47f6709be61f
push id34536
push usernbeleuzu@mozilla.com
push dateThu, 30 Aug 2018 16:57:50 +0000
treeherdermozilla-central@91dc93fd9245 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1486783
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 1486783 - Fix some undefined property references to description and previewImageURL in PlacesUtils.jsm and History.jsm. r=mak Differential Revision: https://phabricator.services.mozilla.com/D4451
toolkit/components/places/History.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/history/test_update.js
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -1510,21 +1510,21 @@ var update = async function(db, pageInfo
   if (typeof pageInfo.guid === "string") {
     whereClauseFragment = "guid = :guid";
     baseParams.guid = pageInfo.guid;
   } else {
     whereClauseFragment = "url_hash = hash(:url) AND url = :url";
     baseParams.url = pageInfo.url.href;
   }
 
-  if (pageInfo.description || pageInfo.description === null) {
+  if ("description" in pageInfo) {
     updateFragments.push("description");
     params.description = pageInfo.description;
   }
-  if (pageInfo.previewImageURL || pageInfo.previewImageURL === null) {
+  if ("previewImageURL" in pageInfo) {
     updateFragments.push("preview_image_url");
     params.preview_image_url = pageInfo.previewImageURL ? pageInfo.previewImageURL.href : null;
   }
   if (updateFragments.length > 0) {
     // Since this data may be written at every visit and is textual, avoid
     // overwriting the existing record if it didn't change.
     await db.execute(`
       UPDATE moz_places
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1057,26 +1057,26 @@ var PlacesUtils = {
     }
 
     if (typeof pageInfo.title === "string") {
       info.title = pageInfo.title;
     } else if (pageInfo.title != null && pageInfo.title != undefined) {
       throw new TypeError(`title property of PageInfo object: ${pageInfo.title} must be a string if provided`);
     }
 
-    if (typeof pageInfo.description === "string" || pageInfo.description === null) {
+    if ("description" in pageInfo && (typeof pageInfo.description === "string" || pageInfo.description === null)) {
       info.description = pageInfo.description ? pageInfo.description.slice(0, DB_DESCRIPTION_LENGTH_MAX) : null;
     } else if (pageInfo.description !== undefined) {
       throw new TypeError(`description property of pageInfo object: ${pageInfo.description} must be either a string or null if provided`);
     }
 
-    if (pageInfo.previewImageURL || pageInfo.previewImageURL === null) {
+    if ("previewImageURL" in pageInfo) {
       let previewImageURL = pageInfo.previewImageURL;
 
-      if (previewImageURL === null) {
+      if (!previewImageURL) {
         info.previewImageURL = null;
       } else if (typeof(previewImageURL) === "string" && previewImageURL.length <= DB_URL_LENGTH_MAX) {
         info.previewImageURL = new URL(previewImageURL);
       } else if (previewImageURL instanceof Ci.nsIURI && previewImageURL.spec.length <= DB_URL_LENGTH_MAX) {
         info.previewImageURL = new URL(previewImageURL.spec);
       } else if (previewImageURL instanceof URL && previewImageURL.href.length <= DB_URL_LENGTH_MAX) {
         info.previewImageURL = previewImageURL;
       } else {
--- a/toolkit/components/places/tests/history/test_update.js
+++ b/toolkit/components/places/tests/history/test_update.js
@@ -153,16 +153,21 @@ add_task(async function test_previewImag
   previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
   Assert.strictEqual(null, previewImageURLInDB, "a null previewImageURL should set it to null in the database");
 
   let guid = await PlacesTestUtils.fieldInDB(TEST_URL, "guid");
   previewImageURL = IMAGE_URL;
   await PlacesUtils.history.update({ url: TEST_URL, guid, previewImageURL });
   previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
   Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via GUID as expected");
+
+  previewImageURL = "";
+  await PlacesUtils.history.update({ url: TEST_URL, previewImageURL});
+  previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+  Assert.strictEqual(null, previewImageURLInDB, "an empty previewImageURL should set it to null in the database");
 });
 
 add_task(async function test_change_description_and_preview_saved() {
   await PlacesUtils.history.clear();
 
   let TEST_URL = "http://mozilla.org/test_change_both_saved";
   await PlacesTestUtils.addVisits(TEST_URL);
   Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL));