Improve showing comment changes to user

Add setting for including edits to existing comments in the number unseen comments in the navigation panel. Verify that diffs are not empty before showing the Diff link.

Change-Id: Ibb85c41e63a082fd24b9376ff3a7a190f244da27

i18n/en.json

@@ -348,6 +348,7 @@
   "sd-autopreview": "Preview the comment as I type",
   "sd-collapsethreadslevel": "Autocollapse threads at level",
   "sd-collapsethreadslevel-help": "0 to never autocollapse.",
+  "sd-counteditsasnewcomments": "Include edits to existing comments in the number of unseen comments in the navigation panel",
   "sd-desktopnotifications": "Desktop notifications",
   "sd-desktopnotifications-radio-all": "Notify me about replies to my comments and comments in topic I'm {{gender:$1|subscribed to}}",
   "sd-desktopnotifications-radio-tome": "Notify me about replies to my comments only",

i18n/qqq.json

@@ -329,6 +329,7 @@
 	"sd-autopreview": "Label of the checkbox in the settings dialog.",
 	"sd-collapsethreadslevel": "The number input follows the label (for example, \"Autocollapse threads at level... 10\"). The user is supposed to set the level of comments.\n\n----\nLabel of the number input in the settings dialog.",
 	"sd-collapsethreadslevel-help": "It means \"Set the setting value to 0 to never autocollapse comment threads\".\n\nHelp text for the number input labeled with the {{msg-wm|Convenient-discussions-sd-collapsethreadslevel}} message in the script settings dialog.",
+	"sd-counteditsasnewcomments": "\"Include\" here is [[wikt:imperfective aspect|imperfective]]: include each time.\n\n----\nLabel of the checkbox in the settings dialog.",
 	"sd-desktopnotifications": "Label of the radio select in the settings dialog.",
 	"sd-desktopnotifications-radio-all": "Label of the item of the radio select labeled with the {{msg-wm|Convenient-discussions-sd-desktopnotifications}} message in the settings dialog.\n\n* $1 (optional): [[mw:Manual:Messages API#GENDER in JavaScript|the user object]], for use with <nowiki>{{gender:}}</nowiki> (<code><nowiki>{{gender:</nowiki>$2|''male text''|''female text''|''text for unspecified''}}</code>).",
 	"sd-desktopnotifications-radio-tome": "Label of the item of the radio select labeled with the {{msg-wm|Convenient-discussions-sd-desktopnotifications}} message in the settings dialog.",

i18n/ru.json

@@ -331,6 +331,7 @@
 	"sd-autopreview": "Предпросматривать сообщение по мере набора",
 	"sd-collapsethreadslevel": "Автоматически сворачивать ветки, начиная с уровня",
 	"sd-collapsethreadslevel-help": "Установите значение 0, чтобы не сворачивать ветки автоматически.",
+	"sd-counteditsasnewcomments": "Включать правки существующих сообщений в число непросмотренных сообщений в навигационной панели",
 	"sd-desktopnotifications": "Уведомления на рабочий стол",
 	"sd-desktopnotifications-radio-all": "Об ответах на мои сообщения и о сообщениях в разделах, на которые я {{gender:$1|подписан|подписана}}",
 	"sd-desktopnotifications-radio-tome": "Только об ответах на мои сообщения",

src/Comment.js

@@ -44,6 +44,7 @@ class Comment extends CommentSkeleton {
     this.hideTimezone = settings.get('hideTimezone');
     this.timestampFormat = settings.get('timestampFormat');
     this.useUiTime = settings.get('useUiTime');
+    this.countEditsAsNewComments = settings.get('countEditsAsNewComments');
 
     /**
      * Comment author user object.
@@ -2006,34 +2007,17 @@ class Comment extends CommentSkeleton {
    * @private
    */
   async showDiff(comparedRevisionId, commentsData) {
-    let revisionIdLesser = Math.min(mw.config.get('wgRevisionId'), comparedRevisionId);
-    let revisionIdGreater = Math.max(mw.config.get('wgRevisionId'), comparedRevisionId);
-
-    const revisionsRequest = controller.getApi().post({
-      action: 'query',
-      revids: [revisionIdLesser, revisionIdGreater],
-      prop: 'revisions',
-      rvslots: 'main',
-      rvprop: ['ids', 'content'],
-      redirects: !mw.config.get('wgIsRedirect'),
-    }).catch(handleApiReject);
-
-    const compareRequest = controller.getApi().post({
-      action: 'compare',
-      fromtitle: this.getSourcePage().name,
-      fromrev: revisionIdLesser,
-      torev: revisionIdGreater,
-      prop: ['diff'],
-    }).catch(handleApiReject);
-
-    let [revisionsResp, compareResp] = await Promise.all([
-      revisionsRequest,
-      compareRequest,
+    const revisionIdLesser = Math.min(mw.config.get('wgRevisionId'), comparedRevisionId);
+    const revisionIdGreater = Math.max(mw.config.get('wgRevisionId'), comparedRevisionId);
+
+    let [revisions, body] = await Promise.all([
+      this.getSourcePage().getRevisions({
+        revids: [revisionIdLesser, revisionIdGreater],
+        rvprop: ['content'],
+      }),
+      this.getSourcePage().compareRevisions(revisionIdLesser, revisionIdGreater),
       mw.loader.using(['mediawiki.diff', 'mediawiki.diff.styles']),
     ]);
-
-    const revisions = revisionsResp.query?.pages?.[0]?.revisions;
-    const body = compareResp?.compare?.body;
     if (!revisions || body === undefined) {
       throw new CdError({
         type: 'api',
@@ -2057,43 +2041,53 @@ class Comment extends CommentSkeleton {
           .addClass('cd-commentDiffView-below')
           .append(
             $('<a>')
-              .attr('href', this.getSourcePage().getUrl({
+              .attr('href', cd.page.getUrl({
                 oldid: revisionIdLesser,
                 diff: revisionIdGreater,
               }))
               .attr('target', '_blank')
 
-              // Make it work in https://ru.wikipedia.org/wiki/User:Serhio_Magpie/instantDiffs.js
+              // Make it work in https://commons.wikimedia.org/wiki/User:Serhio_Magpie/instantDiffs.js
               .attr('data-instantdiffs-link', 'link')
 
               .text(cd.s('comment-diff-full')),
             cd.sParse('dot-separator'),
             $('<a>')
-              .attr('href', this.getSourcePage().getUrl({ action: 'history' }))
+              .attr('href', cd.page.getUrl({ action: 'history' }))
               .attr('target', '_blank')
               .text(cd.s('comment-diff-history'))
           )
       )
       .children();
-    mw.hook('wikipage.content').fire($message);
     OO.ui.alert($message, {
       title: cd.s('comment-diff-title'),
       size: 'larger',
     });
+
+    // FIXME: "wikipage.content hook should not be fired on unattached content".
+    mw.hook('wikipage.content').fire($message);
   }
 
   /**
    * Update the comment's properties, add a small text next to the signature saying the comment has
    * been changed or deleted, and change the comment's styling if it has been.
    *
    * @param {'changed'|'changedSince'|'deleted'} type Type of the mark.
-   * @param {boolean} [isNewVersionRendered] Has the new version of the comment been rendered.
+   * @param {boolean} [isNewVersionRendered] Is the new version of the comment rendered
+   *   (successfully updated or, for `changedSince` type, has been a new one from the beginning).
    * @param {number} [comparedRevisionId] ID of the revision to compare with when the user clicks to
    *   see the diff.
    * @param {object} [commentsData] Data of the comments as of the current revision and the revision
    *   to compare with.
-   */
-  markAsChanged(type, isNewVersionRendered, comparedRevisionId, commentsData) {
+   * @param {boolean} [showDiffLink=true] Whether to show the diff link if it makes sense.
+   */
+  async markAsChanged(
+    type,
+    isNewVersionRendered,
+    comparedRevisionId,
+    commentsData,
+    showDiffLink = true
+  ) {
     let stringName;
     switch (type) {
       case 'changed':
@@ -2122,8 +2116,7 @@ class Comment extends CommentSkeleton {
         },
       });
 
-    const diffLink = type === 'deleted' || this.getSourcePage() !== cd.page ?
-      undefined :
+    const diffLink = showDiffLink && this.getSourcePage().isCurrent() && type !== 'deleted' ?
       new Button({
         label: cd.s('comment-diff'),
         action: async () => {
@@ -2140,11 +2133,12 @@ class Comment extends CommentSkeleton {
                 text += ' ' + cd.sParse('error-network');
               }
             }
-            mw.notify(wrapHtml(text), { type: e.data?.code === 'emptyDiff'? 'info' : 'error' });
+            mw.notify(wrapHtml(text), { type: e.data?.code === 'emptyDiff' ? 'info' : 'error' });
           }
           diffLink.setPending(false);
         },
-      });
+      }) :
+      undefined;
 
     let refreshLinkSeparator;
     let diffLinkSeparator;
@@ -2194,6 +2188,10 @@ class Comment extends CommentSkeleton {
       this.flashChangedOnSight();
     }
 
+    if (this.countEditsAsNewComments && (type === 'changed' || type === 'changedSince')) {
+      this.isSeen = false;
+    }
+
     // Layers are supposed to be updated (deleted comments background, repositioning) separately,
     // see updateChecker~checkForNewChanges(), for example.
   }

src/pageRegistry.js

@@ -525,7 +525,7 @@ export class Page {
   async getRevisions(customOptions = {}, inBackground = false) {
     const options = Object.assign({}, {
       action: 'query',
-      titles: this.name,
+      titles: customOptions.revids ? undefined : this.name,
       rvslots: 'main',
       prop: 'revisions',
       redirects: !(this.isCurrent() && mw.config.get('wgIsRedirect')),
@@ -927,6 +927,23 @@ export class Page {
     return this;
   }
 
+  /**
+   * Get a diff between two revisions of the page.
+   *
+   * @param {number} revisionIdFrom
+   * @param {number} revisionIdTo
+   * @returns {Promise.<string>}
+   */
+  async compareRevisions(revisionIdFrom, revisionIdTo) {
+    return (await controller.getApi().post({
+      action: 'compare',
+      fromtitle: this.name,
+      fromrev: revisionIdFrom,
+      torev: revisionIdTo,
+      prop: ['diff'],
+    }).catch(handleApiReject))?.compare?.body;
+  }
+
   /**
    * Set some map object variables related to archive pages.
    *

src/settings.js

@@ -78,6 +78,7 @@ export default {
 
       'autopreview': true,
       'collapseThreadsLevel': 10,
+      'countEditsAsNewComments': false,
       'desktopNotifications': 'unknown',
       'enableThreads': true,
       'hideTimezone': false,
@@ -182,11 +183,16 @@ export default {
             name: 'highlightNewInterval',
             type: 'number',
             min: 0,
-            max: 99999999,
+            max: 9999999,
             buttonStep: 5,
             label: cd.s('sd-highlightnewinterval'),
             help: cd.s('sd-highlightnewinterval-help'),
           },
+          {
+            name: 'countEditsAsNewComments',
+            type: 'checkbox',
+            label: cd.s('sd-counteditsasnewcomments'),
+          },
           {
             name: 'improvePerformance',
             type: 'checkbox',

src/updateChecker.js

@@ -345,7 +345,7 @@ async function checkForUpdates() {
           mapSections(sections);
           mapComments(currentComments, newComments);
 
-          updateChecker.emit('sectionsUpdate', sections)
+          updateChecker.emit('sectionsUpdate', sections);
 
           // We check for changes before notifying about new comments to notify about changes in a
           // renamed section if it is watched.
@@ -406,6 +406,7 @@ function checkForChangesSincePreviousVisit(currentComments, submittedCommentId)
   const seen = seenStorageItem.get(mw.config.get('wgArticleId'));
 
   const changeList = [];
+  const markAsChangedData = [];
   currentComments.forEach((currentComment) => {
     if (currentComment.id === submittedCommentId) return;
 
@@ -427,7 +428,12 @@ function checkForChangesSincePreviousVisit(currentComments, submittedCommentId)
           1: currentComment,
         };
 
-        comment.markAsChanged('changedSince', true, previousVisitRevisionId, commentsData);
+        markAsChangedData.push({
+          comment,
+          isNewRevisionRendered: true,
+          comparedRevisionId: previousVisitRevisionId,
+          commentsData,
+        });
 
         if (comment.isOpeningSection) {
           comment.section?.resubscribeIfRenamed(currentComment, oldComment);
@@ -438,6 +444,13 @@ function checkForChangesSincePreviousVisit(currentComments, submittedCommentId)
     }
   });
 
+  markCommentsAsChanged(
+    'changedSince',
+    markAsChangedData,
+    previousVisitRevisionId,
+    mw.config.get('wgRevisionId')
+  );
+
   if (changeList.length) {
     /**
      * Existing comments have changed since the previous visit.
@@ -464,12 +477,13 @@ function checkForChangesSincePreviousVisit(currentComments, submittedCommentId)
  */
 function checkForNewChanges(currentComments) {
   const changeList = [];
+  const markAsChangedData = [];
   currentComments.forEach((currentComment) => {
     const newComment = currentComment.match;
     let comment;
     const events = {};
 
-    // Different indexes to supply one object both to the event and Comment#markAsChanged.
+    // Different indexes to supply one object both to the event and Comment#markAsChanged().
     const commentsData = {
       current: currentComment,
       new: newComment,
@@ -494,7 +508,12 @@ function checkForNewChanges(currentComments) {
           // Comment#flashChanged() called indirectly by Comment#markAsChanged().
           comment.htmlToCompare = newComment.htmlToCompare;
 
-          comment.markAsChanged('changed', updateSuccess, lastCheckedRevisionId, commentsData);
+          markAsChangedData.push({
+            comment,
+            isNewRevisionRendered: updateSuccess,
+            comparedRevisionId: lastCheckedRevisionId,
+            commentsData,
+          });
           events.changed = { updateSuccess };
         }
       } else if (comment.isChanged) {
@@ -515,6 +534,13 @@ function checkForNewChanges(currentComments) {
     }
   });
 
+  markCommentsAsChanged(
+    'changed',
+    markAsChangedData,
+    mw.config.get('wgRevisionId'),
+    lastCheckedRevisionId
+  );
+
   if (changeList.length) {
     updateChecker.emit('newChanges', changeList);
 
@@ -537,6 +563,69 @@ function checkForNewChanges(currentComments) {
   }
 }
 
+/**
+ * Data needed to mark the comment as changed
+ *
+ * @typedef {object} MarkAsChangedData
+ * @property {import('./Comment').default} comment
+ * @property {boolean} updateSuccess
+ * @property {object} commentsData
+ * @private
+ */
+
+/**
+ * Mark comments as changed, verifying diffs if possible to decide whether to show the diff link.
+ *
+ * @param {'changed'|'changedSince'|'deleted'} type
+ * @param {MarkAsChangedData[]} data
+ * @param {number} revisionIdLesser
+ * @param {number} revisionIdGreater
+ */
+async function markCommentsAsChanged(type, data, revisionIdLesser, revisionIdGreater) {
+  if (!data.length) return;
+
+  const currentRevisionId = mw.config.get('wgRevisionId');
+
+  // Don't process >20 diffs, that's too much and probably means something is broken
+  const verifyDiffs = (
+    data.length <= 20 &&
+    data.some(({ comment }) => comment.getSourcePage().isCurrent())
+  );
+
+  let revisions;
+  let compareBody;
+  if (verifyDiffs) {
+    try {
+      revisions = await cd.page.getRevisions({
+        revids: [revisionIdLesser, revisionIdGreater],
+        rvprop: ['content'],
+      });
+      compareBody = await cd.page.compareRevisions(revisionIdLesser, revisionIdGreater);
+    } catch {
+      // Empty
+    }
+  }
+  if (!isPageStillAtRevision(currentRevisionId)) return;
+
+  data.forEach(({ comment, isNewRevisionRendered, comparedRevisionId, commentsData }) => {
+    comment.markAsChanged(
+      type,
+      isNewRevisionRendered,
+      comparedRevisionId,
+      commentsData,
+      verifyDiffs && compareBody !== undefined && revisions !== undefined ?
+        Boolean(
+          comment.scrubDiff(compareBody, revisions, commentsData)
+            .find('.diff-deletedline, .diff-addedline')
+            .length
+        ) :
+        true
+    );
+  });
+
+  commentRegistry.emit('registerSeen');
+}
+
 /**
  * Check if the page is still at the specified revision and nothing is loading.
  *