Bug 1892890 - Change default value for `includeFolders` to false in Manifest V3. r=#thunderbird-reviewers

The methods to retrieve accounts and folders may return a lot of folder
and subfolder data. In review I saw that most add-ons do not use the
information and it may have unnecessary performance implications.

The initial implementation always returned all folders and the optional
flag was added only later after such performance implications have been
reported. We however could not change the default in Manifest V2 without
breaking add-ons. Doing that now for Manifest V3.

Differential Revision: https://phabricator.services.mozilla.com/D208312

--HG--
extra : rebase_source : b82e5eada4906752ebd26f03e18637d0cfd7ee6a

mail/components/extensions/schemas/accounts.json

@@ -88,15 +88,45 @@
       {
         "name": "list",
         "type": "function",
+        "min_manifest_version": 3,
+        "description": "Returns all mail accounts. They will be returned in the same order as used in Thunderbird's folder pane.",
+        "async": "callback",
+        "parameters": [
+          {
+            "name": "includeFolders",
+            "optional": true,
+            "default": false,
+            "type": "boolean",
+            "description": "Specifies whether the returned :ref:`accounts.MailAccount` objects should included their account's folders. Defaults to <value>false</value>."
+          },
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "type": "array",
+                "items": {
+                  "$ref": "accounts.MailAccount"
+                }
+              }
+            ]
+          }
+        ]
+      },
+      {
+        "name": "list",
+        "type": "function",
+        "max_manifest_version": 2,
         "description": "Returns all mail accounts. They will be returned in the same order as used in Thunderbird's folder pane.",
         "async": "callback",
         "parameters": [
           {
             "name": "includeFolders",
-            "description": "Specifies whether the returned :ref:`accounts.MailAccount` objects should included their account's folders. Defaults to <value>true</value>.",
             "optional": true,
             "default": true,
-            "type": "boolean"
+            "type": "boolean",
+            "description": "Specifies whether the returned :ref:`accounts.MailAccount` objects should included their account's folders. Defaults to <value>true</value>."
           },
           {
             "type": "function",
@@ -116,6 +146,38 @@
       {
         "name": "get",
         "type": "function",
+        "min_manifest_version": 3,
+        "description": "Returns details of the requested account, or <value>null</value> if it doesn't exist.",
+        "async": "callback",
+        "parameters": [
+          {
+            "name": "accountId",
+            "$ref": "MailAccountId"
+          },
+          {
+            "name": "includeFolders",
+            "optional": true,
+            "default": false,
+            "type": "boolean",
+            "description": "Specifies whether the returned :ref:`accounts.MailAccount` object should included the account's folders. Defaults to <value>false</value>."
+          },
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "$ref": "accounts.MailAccount",
+                "optional": true
+              }
+            ]
+          }
+        ]
+      },
+      {
+        "name": "get",
+        "type": "function",
+        "max_manifest_version": 2,
         "description": "Returns details of the requested account, or <value>null</value> if it doesn't exist.",
         "async": "callback",
         "parameters": [
@@ -125,10 +187,10 @@
           },
           {
             "name": "includeFolders",
-            "description": "Specifies whether the returned :ref:`accounts.MailAccount` object should included the account's folders. Defaults to <value>true</value>.",
             "optional": true,
             "default": true,
-            "type": "boolean"
+            "type": "boolean",
+            "description": "Specifies whether the returned :ref:`accounts.MailAccount` object should included the account's folders. Defaults to <value>true</value>."
           },
           {
             "type": "function",

mail/components/extensions/schemas/addressBook.json

@@ -343,25 +343,25 @@
             "type": "boolean",
             "optional": true,
             "default": true,
-            "description": "Whether to include results from local address books. Defaults to true."
+            "description": "Whether to include results from local address books. Defaults to <value>true</value>."
           },
           "includeRemote": {
             "type": "boolean",
             "optional": true,
             "default": true,
-            "description": "Whether to include results from remote address books. Defaults to true."
+            "description": "Whether to include results from remote address books. Defaults to <value>true</value>."
           },
           "includeReadOnly": {
             "type": "boolean",
             "optional": true,
             "default": true,
-            "description": "Whether to include results from read-only address books. Defaults to true."
+            "description": "Whether to include results from read-only address books. Defaults to <value>true</value>."
           },
           "includeReadWrite": {
             "type": "boolean",
             "optional": true,
             "default": true,
-            "description": "Whether to include results from read-write address books. Defaults to true."
+            "description": "Whether to include results from read-write address books. Defaults to <value>true</value>."
           }
         }
       },

mail/components/extensions/schemas/folders.json

@@ -445,19 +445,50 @@
       {
         "name": "get",
         "type": "function",
+        "min_manifest_version": 3,
         "description": "Returns the specified folder.",
         "async": "callback",
         "parameters": [
           {
-            "name": "folder",
+            "name": "folderId",
             "$ref": "folders.MailFolderId"
           },
           {
             "name": "includeSubFolders",
-            "description": "Specifies whether the returned :ref:`folders.MailFolder` object should include all its nested subfolders . Defaults to <value>true</value>.",
             "optional": true,
+            "type": "boolean",
+            "default": false,
+            "description": "Specifies whether the returned :ref:`folders.MailFolder` object should include all its nested subfolders. Defaults to <value>false</value>."
+          },
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "$ref": "folders.MailFolder"
+              }
+            ]
+          }
+        ]
+      },
+      {
+        "name": "get",
+        "type": "function",
+        "max_manifest_version": 2,
+        "description": "Returns the specified folder.",
+        "async": "callback",
+        "parameters": [
+          {
+            "name": "folderId",
+            "$ref": "folders.MailFolderId"
+          },
+          {
+            "name": "includeSubFolders",
+            "optional": true,
+            "type": "boolean",
             "default": true,
-            "type": "boolean"
+            "description": "Specifies whether the returned :ref:`folders.MailFolder` object should include all its nested subfolders. Defaults to <value>true</value>."
           },
           {
             "type": "function",
@@ -787,7 +818,7 @@
           },
           {
             "name": "includeSubFolders",
-            "description": "Specifies whether the returned :ref:`folders.MailFolder` object for each parent folder should include its nested subfolders . Defaults to <value>false</value>.",
+            "description": "Specifies whether the returned :ref:`folders.MailFolder` object for each parent folder should include its nested subfolders. Defaults to <value>false</value>.",
             "optional": true,
             "default": false,
             "type": "boolean"
@@ -810,6 +841,40 @@
       {
         "name": "getSubFolders",
         "type": "function",
+        "min_manifest_version": 3,
+        "description": "Get the subfolders of the specified folder or account.",
+        "async": "callback",
+        "parameters": [
+          {
+            "name": "folderId",
+            "$ref": "folders.MailFolderId"
+          },
+          {
+            "name": "includeSubFolders",
+            "optional": true,
+            "type": "boolean",
+            "default": false,
+            "description": "Specifies whether the returned :ref:`folders.MailFolder` object for each direct subfolder should also include all its nested subfolders. Defaults to <value>false</value>."
+          },
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": [
+              {
+                "type": "array",
+                "items": {
+                  "$ref": "folders.MailFolder"
+                }
+              }
+            ]
+          }
+        ]
+      },
+      {
+        "name": "getSubFolders",
+        "type": "function",
+        "max_manifest_version": 2,
         "description": "Get the subfolders of the specified folder or account.",
         "async": "callback",
         "parameters": [
@@ -820,12 +885,10 @@
                 "$ref": "folders.MailFolderId"
               },
               {
-                "max_manifest_version": 2,
                 "deprecated": "Support deprecated since Thunderbird 121 and removed in Manifest V3: folders.getSubFolders() requires to specify a MailFolderId instead of a full MailFolder object.",
                 "$ref": "folders.MailFolder"
               },
               {
-                "max_manifest_version": 2,
                 "deprecated": "Support deprecated since Thunderbird 121 and removed in Manifest V3: folders.getSubFolders() requires to specify the MailFolderId of the account's rootFolder, instead of a full MailAccount object.",
                 "$ref": "accounts.MailAccount"
               }
@@ -833,10 +896,10 @@
           },
           {
             "name": "includeSubFolders",
-            "description": "Specifies whether the returned :ref:`folders.MailFolder` object for each direct subfolder should also include all its nested subfolders . Defaults to <value>true</value>.",
             "optional": true,
+            "type": "boolean",
             "default": true,
-            "type": "boolean"
+            "description": "Specifies whether the returned :ref:`folders.MailFolder` object for each direct subfolder should also include all its nested subfolders. Defaults to <value>true</value>."
           },
           {
             "type": "function",

mail/components/extensions/test/browser/browser_ext_mailTabs_create.js

@@ -223,7 +223,7 @@ add_task(async function test_create_mv3() {
     }
 
     const [accountId] = await window.waitForMessage();
-    const { rootFolder } = await browser.accounts.get(accountId);
+    const { rootFolder } = await browser.accounts.get(accountId, true);
     const displayedFolder = rootFolder.subFolders[0];
     delete displayedFolder.subFolders;
 

mail/components/extensions/test/browser/browser_ext_mailTabs_mv3.js

@@ -172,7 +172,7 @@ add_task(async function test_update() {
     }
 
     const [accountId] = await window.waitForMessage();
-    const { rootFolder } = await browser.accounts.get(accountId);
+    const { rootFolder } = await browser.accounts.get(accountId, true);
     const folder = rootFolder.subFolders[0];
 
     await browser.mailTabs.update({ displayedFolderId: folder.id });

mail/components/extensions/test/xpcshell/test_ext_accounts_mv3.js

@@ -22,8 +22,8 @@ add_task(async function test_accounts() {
         "The default account should be null, as none is defined."
       );
 
-      // Check that all folders are included by default.
-      const result1 = await browser.accounts.list();
+      // Check that folders are included on request.
+      const result1 = await browser.accounts.list(true);
       browser.test.assertEq(1, result1.length);
       window.assertDeepEqual(
         {
@@ -63,11 +63,21 @@ add_task(async function test_accounts() {
         );
       }
 
+      // Test that excluding folders works by default.
+      const result1WithOutFoldersDefault = await browser.accounts.list();
+      for (const account of result1WithOutFoldersDefault) {
+        browser.test.assertEq(
+          undefined,
+          account.rootFolder.subFolders,
+          "Folders not included"
+        );
+      }
+
       const [account2Id, account2Name] = await window.sendMessage(
         "create account 2"
       );
       // The new account is defined as default and should be returned first.
-      const result2 = await browser.accounts.list();
+      const result2 = await browser.accounts.list(true);
       browser.test.assertEq(2, result2.length);
       window.assertDeepEqual(
         [
@@ -117,9 +127,9 @@ add_task(async function test_accounts() {
         result2
       );
 
-      const result3 = await browser.accounts.get(account1Id);
+      const result3 = await browser.accounts.get(account1Id, true);
       window.assertDeepEqual(result1[0], result3);
-      const result4 = await browser.accounts.get(account2Id);
+      const result4 = await browser.accounts.get(account2Id, true);
       window.assertDeepEqual(result2[0], result4);
 
       const result3WithoutFolders = await browser.accounts.get(
@@ -141,9 +151,26 @@ add_task(async function test_accounts() {
         "Folders not included"
       );
 
+      const result3WithoutFoldersDefault = await browser.accounts.get(
+        account1Id
+      );
+      browser.test.assertEq(
+        undefined,
+        result3WithoutFoldersDefault.rootFolder.subFolders,
+        "Folders not included"
+      );
+      const result4WithoutFoldersDefault = await browser.accounts.get(
+        account2Id
+      );
+      browser.test.assertEq(
+        undefined,
+        result4WithoutFoldersDefault.rootFolder.subFolders,
+        "Folders not included"
+      );
+
       await window.sendMessage("create folders");
 
-      const result5 = await browser.accounts.get(account1Id);
+      const result5 = await browser.accounts.get(account1Id, true);
       const platformInfo = await browser.runtime.getPlatformInfo();
       window.assertDeepEqual(
         [
@@ -182,7 +209,7 @@ add_task(async function test_accounts() {
         await browser.messages.list(folder.id);
       }
 
-      const result6 = await browser.accounts.get(account2Id);
+      const result6 = await browser.accounts.get(account2Id, true);
       window.assertDeepEqual(
         [
           {

mail/components/extensions/test/xpcshell/test_ext_folders_mv3.js

@@ -17,7 +17,7 @@ add_task(
       "background.js": async () => {
         const [accountId, IS_IMAP] = await window.waitForMessage();
 
-        const account = await browser.accounts.get(accountId);
+        const account = await browser.accounts.get(accountId, true);
         const rootFolder = account.rootFolder;
         browser.test.assertEq(3, rootFolder.subFolders.length);
 
@@ -337,7 +337,7 @@ add_task(async function test_FolderInfo_FolderCapabilities_and_favorite() {
         );
       }
 
-      const account = await browser.accounts.get(accountId);
+      const account = await browser.accounts.get(accountId, true);
       const rootFolder = account.rootFolder;
       const folders = await browser.folders.getSubFolders(rootFolder.id, false);
       const InfoTestFolder = folders.find(f => f.name == "InfoTest");
@@ -560,7 +560,7 @@ add_task(
       "background.js": async () => {
         const [accountId] = await window.waitForMessage();
 
-        const account = await browser.accounts.get(accountId);
+        const account = await browser.accounts.get(accountId, true);
         const rootFolder = account.rootFolder;
         const accountFolders = rootFolder.subFolders;
         browser.test.assertEq(
@@ -652,7 +652,7 @@ add_task(
     const files = {
       "background.js": async () => {
         const [accountId] = await window.waitForMessage();
-        const account = await browser.accounts.get(accountId);
+        const account = await browser.accounts.get(accountId, true);
         const rootFolder = account.rootFolder;
 
         // Create a new root folder in the account.
@@ -733,7 +733,14 @@ add_task(
           );
 
           // Check subfolders.
-          testSubFolders(folder.subFolders, expected);
+          browser.test.assertEq(
+            expected.subFolders,
+            !!folder.subFolders,
+            "The folders subFolders should be present as expected."
+          );
+          if (expected.subFolders) {
+            testSubFolders(folder.subFolders, expected);
+          }
 
           // Check parent folders.
           const parents = await browser.folders.getParentFolders(folder.id);
@@ -787,7 +794,7 @@ add_task(
             accountId,
             depth,
             basePath,
-            subFolders: true,
+            subFolders: false,
           });
 
           const subsTrue = await browser.folders.getSubFolders(
@@ -806,7 +813,7 @@ add_task(
             accountId,
             depth,
             basePath,
-            subFolders: true,
+            subFolders: 1, // Only the direct subFolder level is returned.
           });
           testSubFolders(subsFalse, {
             accountId,

mail/components/extensions/test/xpcshell/test_ext_messages_get.js

@@ -197,7 +197,7 @@ add_task(async function test_plain_mv3() {
 
   const extension = ExtensionTestUtils.loadExtension({
     background: async () => {
-      const accounts = await browser.accounts.list();
+      const accounts = await browser.accounts.list(true);
       browser.test.assertEq(1, accounts.length);
 
       for (const account of accounts) {