From 4e3a7fef2456c04f9561b1a67c1be8f9b7e7b28f Mon Sep 17 00:00:00 2001 From: ansuz Date: Fri, 2 Apr 2021 17:14:15 +0530 Subject: [PATCH] add notes from our review --- lib/decrees.js | 7 ++++--- server.js | 7 +++---- www/admin/app-admin.less | 4 ++++ www/admin/inner.js | 12 +++++++----- www/common/application_config_internal.js | 2 +- www/common/notifications.js | 2 +- www/common/outer/async-store.js | 2 +- www/common/outer/mailbox.js | 2 +- www/common/toolbar.js | 2 +- www/support/ui.js | 2 +- 10 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/decrees.js b/lib/decrees.js index 0b5e8572e..9ee294608 100644 --- a/lib/decrees.js +++ b/lib/decrees.js @@ -27,6 +27,7 @@ DISABLE_INTEGRATED_EVICTION // BROADCAST SET_LAST_BROADCAST_HASH SET_SURVEY_URL +SET_MAINTENANCE NOT IMPLEMENTED: @@ -129,10 +130,10 @@ var args_isString = function (args) { return Array.isArray(args) && typeof(args[0]) === "string"; }; var args_isMaintenance = function (args) { - return Array.isArray(args) && args[0] && args[0].end && args[0].start; + return Array.isArray(args) && args[0] && args[0].end && args[0].start; // XXX we could validate that these are numbers && !isNaN }; -var makeBroadcastSetter = function (attr) { +var makeBroadcastSetter = function (attr) { // XXX could pass extra validation here? return function (Env, args) { if (!args_isString(args) && !args_isMaintenance(args)) { throw new Error('INVALID_ARGS'); @@ -149,7 +150,7 @@ var makeBroadcastSetter = function (attr) { commands.SET_LAST_BROADCAST_HASH = makeBroadcastSetter('lastBroadcastHash'); // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_SURVEY_URL', [url]]], console.log) -commands.SET_SURVEY_URL = makeBroadcastSetter('surveyURL'); +commands.SET_SURVEY_URL = makeBroadcastSetter('surveyURL'); // XXX anticipate language-specific surveys // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_MAINTENANCE', [{start: +Date, end: +Date}]]], console.log) // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_MAINTENANCE', [""]]], console.log) diff --git a/server.js b/server.js index 7f4c1fcbf..62e144efa 100644 --- a/server.js +++ b/server.js @@ -113,8 +113,7 @@ var setHeaders = (function () { // Don't set CSP headers on /api/config because they aren't necessary and they cause problems // when duplicated by NGINX in production environments - // XXX /api/broadcast too? - if (/^\/api\/config/.test(req.url)) { return; } + if (/^\/api\/(broadcast|config)/.test(req.url)) { return; } // targeted CSP, generic policies, maybe custom headers const h = [ /^\/common\/onlyoffice\/.*\/index\.html.*/, @@ -273,7 +272,7 @@ var serveConfig = (function () { }; }()); -var serveBroadcast = (function () { +var serveBroadcast = (function () { // XXX deduplicate var cacheString = function () { return (Env.FRESH_KEY? '-' + Env.FRESH_KEY: '') + (Env.DEV_MODE? '-' + (+new Date()): ''); }; @@ -284,7 +283,7 @@ var serveBroadcast = (function () { maintenance = undefined; } return [ - 'define(function(){', + 'define(function(){', // XXX maybe this could just be JSON 'return ' + JSON.stringify({ lastBroadcastHash: Env.lastBroadcastHash, surveyURL: Env.surveyURL, diff --git a/www/admin/app-admin.less b/www/admin/app-admin.less index c3556c869..17450361c 100644 --- a/www/admin/app-admin.less +++ b/www/admin/app-admin.less @@ -14,6 +14,10 @@ display: flex; flex-flow: column; + a { + color: @cryptpad_color_link; + text-decoration: underline; + } .cp-admin-setlimit-form, .cp-admin-broadcast-form { label { diff --git a/www/admin/inner.js b/www/admin/inner.js index 204e39dfc..4642296db 100644 --- a/www/admin/inner.js +++ b/www/admin/inner.js @@ -970,6 +970,7 @@ define([ var getApi = function (cb) { return function () { require(['/api/broadcast?'+ (+new Date())], function (Broadcast) { + // XXX require.s.contexts._ can be used to erase old loaded objects cb(Broadcast); }); }; @@ -1219,7 +1220,7 @@ define([ } if (error) { console.error('One of the selected languages has no data'); - return false; + return false; // XXX better error handling? } return { defaultLanguage: defaultLanguage, @@ -1229,7 +1230,7 @@ define([ var send = function (data) { $button.prop('disabled', 'disabled'); - data.time = +new Date(); + data.time = +new Date(); // XXX not used anymore? common.mailbox.sendTo('BROADCAST_CUSTOM', data, {}, function (err /*, data */) { // XXX unused argument if (err) { $button.prop('disabled', ''); @@ -1249,7 +1250,7 @@ define([ send(data); }); - UI.confirmButton(removeButton, { + UI.confirmButton(removeButton, { // XXX table jank classes: 'btn-danger', }, function () { if (!activeUid) { return; } @@ -1311,7 +1312,8 @@ define([ var end = h('input'); var $start = $(start); var $end = $(end); - var endPickr = Flatpickr(end, { + // XXX new Date().toLocaleString('fr-fr', {month: 'long'}).replace(/./, c => c.toUpperCase()) + var endPickr = Flatpickr(end, { // XXX translations? enableTime: true, minDate: new Date() }); @@ -1411,7 +1413,7 @@ define([ common.openUnsafeURL(Broadcast.surveyURL); }); active = h('div.cp-broadcast-active', [ - h('p', a), + h('p', a), // XXX spacing around this element is really cramped removeButton ]); } diff --git a/www/common/application_config_internal.js b/www/common/application_config_internal.js index f2c75c8a1..4ac3554cd 100644 --- a/www/common/application_config_internal.js +++ b/www/common/application_config_internal.js @@ -162,7 +162,7 @@ define(function() { // making it much faster to open new tabs. config.disableWorkers = false; - //config.surveyURL = ""; + //config.surveyURL = ""; // XXX remove this? // Teams are always loaded during the initial loading screen (for the first tab only if // SharedWorkers are available). Allowing users to be members of multiple teams can diff --git a/www/common/notifications.js b/www/common/notifications.js index 0804bacbf..4a26cab44 100644 --- a/www/common/notifications.js +++ b/www/common/notifications.js @@ -466,7 +466,7 @@ define([ return { add: function(common, data) { - console.log(data); + console.log(data); // XXX noise? var type = data.content.msg.type; if (handlers[type]) { diff --git a/www/common/outer/async-store.js b/www/common/outer/async-store.js index 3ee629b6d..9a6a30faf 100644 --- a/www/common/outer/async-store.js +++ b/www/common/outer/async-store.js @@ -3156,7 +3156,7 @@ define([ }); }; - Store.newVersionReload = function () { + Store.newVersionReload = function () { // XXX not used anymore? broadcast([], "NETWORK_RECONNECT"); }; Store.disconnect = function () { diff --git a/www/common/outer/mailbox.js b/www/common/outer/mailbox.js index 2ef42409c..6605a55ee 100644 --- a/www/common/outer/mailbox.js +++ b/www/common/outer/mailbox.js @@ -341,7 +341,7 @@ proxy.mailboxes = { }; var notify = box.ready; Handlers.add(ctx, box, message, function (dismissed, toDismiss, setAsLKH) { - if (setAsLKH) { + if (setAsLKH) { // XXX confirm whether this if is used? // Update LKH box.data.lastKnownHash = hash; box.data.viewed = []; diff --git a/www/common/toolbar.js b/www/common/toolbar.js index d59c9afa6..07eda5b2e 100644 --- a/www/common/toolbar.js +++ b/www/common/toolbar.js @@ -1032,7 +1032,7 @@ MessengerUI, Messages) { Messages.broadcast_maintenance = "A maintenance is planned between {0} and {1}"; // XXX var createMaintenance = function (toolbar, config) { var $notif = toolbar.$top.find('.'+MAINTENANCE_CLS); - var button = h('button.cp-maintenance-wrench.fa.fa-wrench'); + var button = h('button.cp-maintenance-wrench.fa.fa-wrench'); // XXX might need some color contrast $notif.append(button); diff --git a/www/support/ui.js b/www/support/ui.js index 1a8f3e440..a2b5da4a8 100644 --- a/www/support/ui.js +++ b/www/support/ui.js @@ -445,7 +445,7 @@ define([ }; ctx.FM = common.createFileManager(fmConfig); - ui.send = function (id, type, data, dest) { + ui.send = function (id, type, data, dest) { // XXX confirm that this is actually used return send(ctx, id, type, data, dest); }; ui.sendForm = function (id, form, dest) {