Bug 1740536: Support descriptions for pages. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D130882
2021-11-16 14:54:08 +00:00 · 2021-11-16 14:54:08 +00:00 · 90890337d7
--- a/browser/components/pagedata/OpenGraphPageData.jsm
+++ b/browser/components/pagedata/OpenGraphPageData.jsm
@ -29,10 +29,13 @@ const OpenGraphPageData = {
    let openGraphTags = document.querySelectorAll("meta[property^='og:'");

    for (let tag of openGraphTags) {
-      // Stripping "og:" from the property name.
+      // Strip "og:" from the property name.
      let propertyName = tag.getAttribute("property").substring(3);

      switch (propertyName) {
+        case "description":
+          pageData.description = tag.getAttribute("content");
+          break;
        case "site_name":
          pageData.siteName = tag.getAttribute("content");
          break;
--- a/browser/components/pagedata/SchemaOrgPageData.jsm
+++ b/browser/components/pagedata/SchemaOrgPageData.jsm
@ -98,6 +98,11 @@ function collectProduct(pageData, element) {
    pageData.image = images[0];
  }

+  let descriptions = getProp(element, "description");
+  if (descriptions.length) {
+    pageData.description = descriptions[0];
+  }
+
  pageData.data[PageDataSchema.DATA_TYPE.PRODUCT] = {
    name: getProp(element, "name")[0],
  };
--- a/browser/components/pagedata/schemas/general.schema.json
+++ b/browser/components/pagedata/schemas/general.schema.json
@ -13,6 +13,10 @@
      "description": "The date the data was collected as a timestamp",
      "type": "number"
    },
+    "description": {
+      "description": "A description of the page",
+      "type": "string"
+    },
    "siteName": {
      "description": "A friendly name for the site",
      "type": "string"
--- a/browser/components/pagedata/tests/unit/test_opengraph.js
+++ b/browser/components/pagedata/tests/unit/test_opengraph.js
@ -29,6 +29,8 @@ add_task(async function test_type_website() {
    `,
    {
      siteName: "Mozilla",
+      description:
+        "Mozilla is the not-for-profit behind the lightning fast Firefox browser. We put people over profit to give everyone more power online.",
      image: "https://example.com/preview-image",
      data: {},
    }
@ -57,6 +59,7 @@ add_task(async function test_type_movie() {
    `,
    {
      image: "https://example.com/preview-code-rush",
+      description: "This is the description of the movie.",
      data: {},
    }
  );
--- a/browser/components/pagedata/tests/unit/test_schemaorg.js
+++ b/browser/components/pagedata/tests/unit/test_schemaorg.js
@ -25,11 +25,14 @@ add_task(async function test_single_product_data() {
          <span itemprop="priceCurrency" content="GBP"></span>

          <span itemprop="gtin" content="13572468"></span>
+
+          <span itemprop="description">The most amazing microwave in the world</span>
        </div>
      </body>
      </html>
    `,
    {
+      description: "The most amazing microwave in the world",
      image: BASE_URL + "/bon-echo-microwave-17in.jpg",
      data: {
        [PageDataSchema.DATA_TYPE.PRODUCT]: {
--- a/browser/components/places/Snapshots.jsm
+++ b/browser/components/places/Snapshots.jsm
@ -181,11 +181,20 @@ const Snapshots = new (class Snapshots {
          pageDataIndex++;
        }

+        let { siteName, description, image: previewImageURL } = pageData;
+        let pageInfo = PlacesUtils.validateItemProperties(
+          "PageInfo",
+          PlacesUtils.PAGEINFO_VALIDATORS,
+          { siteName, description, previewImageURL }
+        );
+
        placesBindings[`id${placesIndex}`] = placeId;
-        placesBindings[`site${placesIndex}`] = pageData.siteName ?? null;
-        placesBindings[`image${placesIndex}`] = pageData.image ?? null;
+        placesBindings[`desc${placesIndex}`] = pageInfo.description ?? null;
+        placesBindings[`site${placesIndex}`] = pageInfo.siteName ?? null;
+        placesBindings[`image${placesIndex}`] =
+          pageInfo.previewImageURL?.toString() ?? null;
        placesValues.push(
-          `(:id${placesIndex}, :site${placesIndex}, :image${placesIndex})`
+          `(:id${placesIndex}, :desc${placesIndex}, :site${placesIndex}, :image${placesIndex})`
        );
        placesIndex++;
      } else {
@ -211,11 +220,14 @@ const Snapshots = new (class Snapshots {
        if (placesIndex) {
          await db.execute(
            `
-          WITH pd("place_id", "siteName", "image") AS (
+          WITH pd("place_id", "description", "siteName", "image") AS (
            VALUES ${placesValues.join(", ")}
          )
          UPDATE moz_places
-            SET site_name = pd.siteName, preview_image_url = pd.image
+            SET
+              description = pd.description,
+              site_name = pd.siteName,
+              preview_image_url = pd.image
            FROM pd
            WHERE moz_places.id = pd.place_id;
          `,
@ -408,7 +420,7 @@ const Snapshots = new (class Snapshots {
      `
      SELECT h.url AS url, h.title AS title, created_at, removed_at,
             document_type, first_interaction_at, last_interaction_at,
-             user_persisted, site_name, preview_image_url,
+             user_persisted, description, site_name, preview_image_url,
             group_concat('[' || e.type || ', ' || e.data || ']') AS page_data
             FROM moz_places_metadata_snapshots s
      JOIN moz_places h ON h.id = s.place_id
@ -466,7 +478,7 @@ const Snapshots = new (class Snapshots {
      `
      SELECT h.url AS url, h.title AS title, created_at, removed_at,
             document_type, first_interaction_at, last_interaction_at,
-             user_persisted, site_name, preview_image_url,
+             user_persisted, description, site_name, preview_image_url,
             group_concat('[' || e.type || ', ' || e.data || ']') AS page_data
      FROM moz_places_metadata_snapshots s
      JOIN moz_places h ON h.id = s.place_id
@ -537,6 +549,7 @@ const Snapshots = new (class Snapshots {
      url: row.getResultByName("url"),
      title: row.getResultByName("title"),
      siteName: row.getResultByName("site_name"),
+      description: row.getResultByName("description"),
      image: row.getResultByName("preview_image_url"),
      createdAt: this.#toDate(row.getResultByName("created_at")),
      removedAt: this.#toDate(row.getResultByName("removed_at")),
--- a/browser/components/places/tests/unit/interactions/test_snapshots_pagedata.js
+++ b/browser/components/places/tests/unit/interactions/test_snapshots_pagedata.js
@ -19,6 +19,7 @@ add_task(async function pagedata() {
    url: TEST_URL1,
    date: Date.now(),
    siteName: "Mozilla",
+    description: "We build the Firefox web browser",
    data: {
      [PageDataSchema.DATA_TYPE.PRODUCT]: {
        price: {
@ -94,6 +95,7 @@ add_task(async function pagedata() {

  let snap = await Snapshots.get(TEST_URL1);
  Assert.equal(snap.siteName, "Mozilla", "Should have the site name.");
+  Assert.equal(snap.description, "We build the Firefox web browser");
  Assert.equal(snap.pageData.size, 2, "Should have some page data.");
  Assert.deepEqual(
    snap.pageData.get(PageDataSchema.DATA_TYPE.PRODUCT),
@ -103,6 +105,7 @@ add_task(async function pagedata() {

  await Snapshots.add({ url: TEST_URL2 });
  snap = await Snapshots.get(TEST_URL2);
+  Assert.equal(snap.description, null);
  Assert.equal(snap.pageData.size, 1, "Should have some page data.");
  Assert.deepEqual(
    snap.pageData.get(PageDataSchema.DATA_TYPE.PRODUCT),
@ -148,6 +151,7 @@ add_task(async function pagedata() {
  await Snapshots.add({ url: TEST_URL1, userPersisted: true });
  snap = await Snapshots.get(TEST_URL1);
  Assert.equal(snap.siteName, "Mozilla", "Should have the site name.");
+  Assert.equal(snap.description, "We build the Firefox web browser");
  Assert.equal(snap.pageData.size, 2, "Should have some page data.");
  Assert.deepEqual(
    snap.pageData.get(PageDataSchema.DATA_TYPE.PRODUCT),
@ -157,3 +161,34 @@ add_task(async function pagedata() {

  await reset();
 });
+
+add_task(async function pagedata_validation() {
+  // Register some page data.
+  PageDataService.pageDataDiscovered({
+    url: TEST_URL1,
+    date: Date.now(),
+    siteName:
+      "This is a very long site name that will be truncated when saved to the database",
+    description: "long description".repeat(20),
+    data: {},
+  });
+
+  await addInteractions([
+    {
+      url: TEST_URL1,
+      totalViewTime: 40000,
+      created_at: Date.now() - 1000,
+    },
+  ]);
+
+  await Snapshots.add({ url: TEST_URL1 });
+  let snap = await Snapshots.get(TEST_URL1);
+  Assert.equal(
+    snap.siteName,
+    "This is a very long site name that will be truncat"
+  );
+  Assert.equal(snap.description, "long description".repeat(16));
+  Assert.equal(snap.pageData.size, 0, "Should have no page data.");
+
+  await reset();
+});
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@ -814,6 +814,7 @@ var PlacesUtils = {
  },

  BOOKMARK_VALIDATORS,
+  PAGEINFO_VALIDATORS,
  SYNC_BOOKMARK_VALIDATORS,
  SYNC_CHANGE_RECORD_VALIDATORS,