From 133a278c9c4c7fea4834aadb2ce688f954c335a3 Mon Sep 17 00:00:00 2001
From: Simon Rozman <simon@rozman.si>
Date: Fri, 10 May 2024 15:38:53 +0200
Subject: [PATCH] service2: Rework proofing notifications and aborting

Notifications were inconsistent in case of errors while checking
grammar.
---
 service2.js | 98 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 69 insertions(+), 29 deletions(-)

diff --git a/service2.js b/service2.js
index 880f26b..2409288 100644
--- a/service2.js
+++ b/service2.js
@@ -1,12 +1,12 @@
 // TODO: Port popup dialog from service.js
-// TODO: Test with <div contenteditable="plaintext-only">
+// TODO: Test with contenteditable="plaintext-only"
 // TODO: Implement <textarea> class
 // TODO: Port CKEditor class from service.js
 
 /**
  * Collection of all grammar checking services in the document
  *
- * We dispatch all window messages to all services registered here.
+ * We dispatch relevant window messages to all services registered here.
  */
 let besServices = []
 
@@ -26,7 +26,6 @@ window.addEventListener('scroll', () =>
 class BesService {
   constructor(hostElement) {
     this.hostElement = hostElement
-    this.abortController = new AbortController()
     this.createCorrectionPanel()
 
     // Disable browser built-in spell-checker to prevent collision with our grammar markup.
@@ -47,7 +46,6 @@ class BesService {
     this.hostElement.removeEventListener('scroll', this.handleScroll)
     this.hostElement.spellcheck = this.originalSpellcheck
     this.clearCorrectionPanel()
-    this.abortController.abort()
   }
 
   /**
@@ -55,6 +53,7 @@ class BesService {
    */
   onStartProofing() {
     this.proofingCount = 0 // Ref-count how many grammar-checking blocks of text are active
+    this.proofingError = null // The first non-fatal error in grammar-checking run
     this.proofingMatches = 0 // Number of grammar mistakes detected in entire grammar-checking run
     this.updateStatusIcon('bes-status-loading', 'Besana preverja pravopis.')
   }
@@ -67,7 +66,9 @@ class BesService {
   }
 
   /**
-   * Called when grammar-checking of a block of text failed (as 500 Internal server error, timeout, etc.)
+   * Called when grammar-checking failed (as 500 Internal server error, timeout, etc.)
+   *
+   * This error is fatal and proofing will not continue.
    *
    * @param {Response} response HTTP response
    */
@@ -84,11 +85,8 @@ class BesService {
    * @param {Error} error Error
    */
   onFailedProofingResult(error) {
-    this.proofingCount--
-    this.updateStatusIcon(
-      'bes-status-error',
-      `Pri obdelavi odgovora pravopisnega strežnika je prišlo do napake: ${error}`
-    )
+    if (!this.proofingError) this.proofingError = error
+    if (--this.proofingCount <= 0) this.onEndProofing()
   }
 
   /**
@@ -98,16 +96,24 @@ class BesService {
    */
   onProofingProgress(numberOfMatches) {
     this.proofingMatches += numberOfMatches
-    if (--this.proofingCount <= 0) {
-      // This was the last block of text in the run we were waiting for.
-      // TODO: If onFailedProofingResult was called on a non-last block of text, the below will override the status icon error state.
-      if (this.proofingMatches > 0)
-        this.updateStatusIcon(
-          'bes-status-mistakes',
-          'Število napak: ' + this.proofingMatches
-        )
-      else this.updateStatusIcon('bes-status-success', 'V besedilu ni napak.')
-    }
+    if (--this.proofingCount <= 0) this.onEndProofing()
+  }
+
+  /**
+   * Called when grammar-checking run is ended
+   */
+  onEndProofing() {
+    if (this.proofingError) {
+      this.updateStatusIcon(
+        'bes-status-error',
+        `Pri obdelavi odgovora pravopisnega strežnika je prišlo do napake: ${this.proofingError}`
+      )
+    } else if (this.proofingMatches > 0)
+      this.updateStatusIcon(
+        'bes-status-mistakes',
+        `Število napak: ${this.proofingMatches}`
+      )
+    else this.updateStatusIcon('bes-status-success', 'V besedilu ni napak.')
   }
 
   /**
@@ -255,9 +261,39 @@ class BesDOMService extends BesService {
   unregister() {
     this.hostElement.removeEventListener('input', this.handleInput)
     this.hostElement.removeEventListener('beforeinput', this.handleBeforeInput)
+    if (this.timer) clearTimeout(this.timer)
+    if (this.abortController) this.abortController.abort()
     super.unregister()
   }
 
+  /**
+   * Called initially when grammar-checking run is started
+   */
+  onStartProofing() {
+    super.onStartProofing()
+    this.abortController = new AbortController()
+  }
+
+  /**
+   * Called when grammar-checking failed (as 500 Internal server error, timeout, etc.)
+   *
+   * This error is fatal and proofing will not continue.
+   *
+   * @param {Response} response HTTP response
+   */
+  onFailedProofing(response) {
+    delete this.abortController
+    super.onFailedProofing(response)
+  }
+
+  /**
+   * Called when grammar-checking run is ended
+   */
+  onEndProofing() {
+    delete this.abortController
+    super.onEndProofing()
+  }
+
   /**
    * Called to report scrolling
    */
@@ -293,9 +329,8 @@ class BesDOMService extends BesService {
    * @param {InputEvent} event The event notifying the user of editable content changes
    */
   onBeforeInput(event) {
-    // Abort running grammar checking ASAP.
     if (this.timer) clearTimeout(this.timer)
-    this.abortController.abort()
+    if (this.abortController) this.abortController.abort()
 
     // Remove markup of all blocks of text that are about to change.
     let blockElements = new Set()
@@ -320,7 +355,6 @@ class BesDOMService extends BesService {
 
     // Defer grammar-checking to reduce stress on grammar-checking server.
     this.timer = setTimeout(() => {
-      this.abortController = new AbortController()
       this.proofAll()
       delete this.timer
     }, 1000)
@@ -331,16 +365,22 @@ class BesDOMService extends BesService {
    */
   proofAll() {
     this.onStartProofing()
-    this.proofNode(this.hostElement)
+    this.proofNode(this.hostElement, this.abortController)
+    if (this.proofingCount == 0) {
+      // No text blocks were discovered for proofing. onProofingProgress() will not be called
+      // and we need to notify manually.
+      this.onEndProofing()
+    }
   }
 
   /**
    * Recursively grammar-checks a DOM node.
    *
    * @param {Node} node DOM root node to check
+   * @param {AbortController} abortController Abort controller to cancel grammar-checking
    * @returns {Array} Markup of text to check using BesStr
    */
-  proofNode(node) {
+  proofNode(node, abortController) {
     switch (node.nodeType) {
       case Node.TEXT_NODE:
         return [{ text: node.textContent, node: node, markup: false }]
@@ -353,12 +393,11 @@ class BesDOMService extends BesService {
 
           let data = []
           for (const el2 of node.childNodes)
-            data = data.concat(this.proofNode(el2))
+            data = data.concat(this.proofNode(el2, abortController))
           if (data.some(x => !x.markup && !/^\s*$/.test(x.text))) {
             // Block element contains some text.
             this.onProofing()
-            // Save the abort signal reference. It will change on grammar-check re-start.
-            const signal = this.abortController.signal
+            const signal = abortController.signal
             fetch(
               new Request(besUrl + '/check', {
                 method: 'POST',
@@ -448,7 +487,7 @@ class BesDOMService extends BesService {
           // Inline elements require no markup. Keep plain text only.
           let data = []
           for (const el2 of node.childNodes)
-            data = data.concat(this.proofNode(el2))
+            data = data.concat(this.proofNode(el2, abortController))
           return data
         }
 
@@ -693,6 +732,7 @@ class BesDOMService extends BesService {
 // Auto-register all elements with bes-service class.
 window.addEventListener('load', () => {
   document.querySelectorAll('.bes-service').forEach(hostElement => {
+    // TODO: Treat contenteditable="plaintext-only" separately. It requires manual paragraph splitting on \n\n.
     if (hostElement.tagName === 'TEXTAREA') BesTAService.register(hostElement)
     else BesDOMService.register(hostElement)
   })