From e3a47e48f9ee2391fb4e60c97eb7137855e580af Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Fri, 22 Jan 2021 02:06:06 -0500
Subject: [PATCH] contentcollector: Fix collectContentLineText hook

Before, the hook always ignored the return values provided by the hook
functions. Now the hook functions can change the text by either
returning a string or setting `context.text` to the desired value.

Also drop the `styl` and `cls` context properties. They were never
documented and they were always null.
---
 doc/api/hooks_client-side.md      | 15 ++++++++++++++-
 src/static/js/contentcollector.js | 24 +++++++-----------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/doc/api/hooks_client-side.md b/doc/api/hooks_client-side.md
index 478f5d0a..e87536cf 100755
--- a/doc/api/hooks_client-side.md
+++ b/doc/api/hooks_client-side.md
@@ -421,7 +421,20 @@ Things in context:
 4. text - the text for that line
 
 This hook allows you to validate/manipulate the text before it's sent to the
-server side. The return value should be the validated/manipulated text.
+server side. To change the text, either:
+
+* Set the `text` context property to the desired value and return `undefined`.
+* (Deprecated) Return a string. If a hook function changes the `text` context
+  property, the return value is ignored. If no hook function changes `text` but
+  multiple hook functions return a string, the first one wins.
+
+Example:
+
+```
+exports.collectContentLineText = (hookName, context) => {
+  context.text = tweakText(context.text);
+};
+```
 
 ## collectContentLineBreak
 
diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js
index b20c2583..357b5019 100644
--- a/src/static/js/contentcollector.js
+++ b/src/static/js/contentcollector.js
@@ -338,24 +338,14 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
     _reachBlockPoint(node, 0, state);
 
     if (dom.isTextNode(node)) {
-      let txt = dom.nodeValue(node);
       const tname = dom.getAttribute(node.parentNode, 'name');
-
-      const txtFromHook = hooks.callAll('collectContentLineText', {
-        cc: this,
-        state,
-        tname,
-        node,
-        text: txt,
-        styl: null,
-        cls: null,
-      });
-
-      if (typeof (txtFromHook) === 'object') {
-        txt = dom.nodeValue(node);
-      } else if (txtFromHook) {
-        txt = txtFromHook;
-      }
+      const context = {cc: this, state, tname, node, text: dom.nodeValue(node)};
+      // Hook functions may either return a string (deprecated) or modify context.text. If any hook
+      // function modifies context.text then all returned strings are ignored. If no hook functions
+      // modify context.text, the first hook function to return a string wins.
+      const [hookTxt] =
+          hooks.callAll('collectContentLineText', context).filter((s) => typeof s === 'string');
+      let txt = context.text === dom.nodeValue(node) && hookTxt != null ? hookTxt : context.text;
 
       let rest = '';
       let x = 0; // offset into original text