From 2019e1d70a75dd8c36c8b5b1459e34f3f59d3b41 Mon Sep 17 00:00:00 2001
From: ansuz <ansuz@transitiontech.ca>
Date: Mon, 26 Jul 2021 13:05:47 +0530
Subject: [PATCH] implement minor fixes and add comments following code review

---
 www/common/drive-ui.js               |  7 +++---
 www/common/outer/mailbox-handlers.js |  3 ++-
 www/common/outer/userObject.js       | 32 +++++++++++++++++-----------
 www/teams/main.js                    |  2 ++
 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/www/common/drive-ui.js b/www/common/drive-ui.js
index 9c0a37f42..cd456fd41 100644
--- a/www/common/drive-ui.js
+++ b/www/common/drive-ui.js
@@ -42,7 +42,8 @@ define([
     Pages)
 {
 
-    Messages.fm_link = "Link"; // XXX
+    Messages.fm_link = "Link"; // XXX "New Link" ?
+    // XXX check for all occurrences of `fm_link` before changing it
 
     var APP = window.APP = {
         editable: false,
@@ -1924,10 +1925,10 @@ define([
             var $name = $('<span>', {'class': 'cp-app-drive-element-name'}).text(name);
             $element.append($name);
             if (getViewMode() === 'grid') {
-                $element.attr('title', name);
+                $element.attr('title', name); // XXX Util.fixHTML
             }
 
-            var type = Messages.fm_link;
+            var type = Messages.fm_link; // XXX new translation key ("Link")
             var $type = $('<span>', {
                 'class': 'cp-app-drive-element-type cp-app-drive-element-list'
             }).text(type);
diff --git a/www/common/outer/mailbox-handlers.js b/www/common/outer/mailbox-handlers.js
index bb19d2d91..88e43352b 100644
--- a/www/common/outer/mailbox-handlers.js
+++ b/www/common/outer/mailbox-handlers.js
@@ -238,7 +238,8 @@ define([
         // content.name, content.title, content.href, content.password
 
         if (isMuted(ctx, data)) { return void cb(true); }
-
+        // if the shared content is a 'link' then we can't use the channel to deduplicate notifications
+        // use href instead.
         var channel = content.isStatic ? content.href : Hash.hrefToHexChannelId(content.href, content.password);
         var parsed = Hash.parsePadUrl(content.href);
         var mode = parsed.hashData && parsed.hashData.mode || 'n/a';
diff --git a/www/common/outer/userObject.js b/www/common/outer/userObject.js
index 97c2207e0..4051c79c1 100644
--- a/www/common/outer/userObject.js
+++ b/www/common/outer/userObject.js
@@ -616,13 +616,18 @@ define([
                 var element = elem || files[ROOT];
                 if (!element) { return console.error("Invalid element in root"); }
                 var nbMetadataFolders = 0;
+                // caching this variables saves a lot of hashmap lookups in this loop
+                var static_data = files[STATIC_DATA];
+                var files_data = files[FILES_DATA];
+                var element_el;
                 for (var el in element) {
-                    if (element[el] === null) {
+                    element_el = element[el];
+                    if (element_el === null) {
                         console.error('element[%s] is null', el);
                         delete element[el];
                         continue;
                     }
-                    if (exp.isFolderData(element[el])) {
+                    if (exp.isFolderData(element_el)) {
                         if (nbMetadataFolders !== 0) {
                             debug("Multiple metadata files in folder");
                             delete element[el];
@@ -630,30 +635,30 @@ define([
                         nbMetadataFolders++;
                         continue;
                     }
-                    if (!exp.isFile(element[el], true) && !exp.isFolder(element[el])) {
-                        debug("An element in ROOT was not a folder nor a file. ", element[el]);
+                    if (!exp.isFile(element_el, true) && !exp.isFolder(element_el)) {
+                        debug("An element in ROOT was not a folder nor a file. ", element_el);
                         delete element[el];
                         continue;
                     }
-                    if (exp.isFolder(element[el])) {
-                        fixRoot(element[el]);
+                    if (exp.isFolder(element_el)) {
+                        fixRoot(element_el);
                         continue;
                     }
-                    if (typeof element[el] === "string") {
+                    if (typeof element_el === "string") {
                         // We have an old file (href) which is not in filesData: add it
                         var id = Util.createRandomInteger();
                         var key = Hash.createChannelId();
-                        files[FILES_DATA][id] = {
-                            href: exp.cryptor.encrypt(element[el]),
+                        files_data[id] = {
+                            href: exp.cryptor.encrypt(element_el),
                             filename: el
                         };
                         element[key] = id;
                         delete element[el];
                     }
-                    if (typeof element[el] === "number") {
-                        var data = files[FILES_DATA][element[el]] || files[STATIC_DATA][element[el]];
+                    if (typeof element_el === "number") {
+                        var data = files_data[element_el] || static_data[element_el];
                         if (!data) {
-                            debug("An element in ROOT doesn't have associated data", element[el], el);
+                            debug("An element in ROOT doesn't have associated data", element_el, el);
                             delete element[el];
                         }
                     }
@@ -862,6 +867,7 @@ define([
                 toClean.forEach(function (id) {
                     spliceFileData(id);
                 });
+                // make sure that links are displayed at least once in your drive if you are going to keep them
                 var sd = files[STATIC_DATA];
                 var toCleanSD = [];
                 for (var id2 in sd) {
@@ -929,7 +935,7 @@ define([
                 }
             };
             var fixStaticData = function () {
-                if (typeof(files[STATIC_DATA]) !== "object") {
+                if (Util.isObject(files[STATIC_DATA])) {
                     debug("STATIC_DATA was not an object");
                     files[STATIC_DATA] = {};
                 }
diff --git a/www/teams/main.js b/www/teams/main.js
index 4bf00e105..7d12126eb 100644
--- a/www/teams/main.js
+++ b/www/teams/main.js
@@ -24,6 +24,8 @@ define([
 
             sframeChan.on('Q_DRIVE_USEROBJECT', function (data, cb) {
                 if (!teamId) { return void cb({error: 'EINVAL'}); }
+                // a teamId of -1 bypasses guards against modifying your drive
+                // from the team app
                 if (data.teamId !== -1) { data.teamId = teamId; }
                 else { delete data.teamId; }
                 Cryptpad.userObjectCommand(data, cb);