1702754 - Calendar "Too much recursion" in ASSERT(), etc

Status: UNCONFIRMED

(SeaMonkey :: Calendar, defect)

Description

[fieldhouse]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:60.0) Gecko/20100101 Firefox/60.0 SeaMonkey/2.53.7

Steps to reproduce:

SM 2.53.7 Linux x86

With the Error Console open, navigate to any IMAP mail folder.

Actual results:

In the Error Console, this appears:

Timestamp: 01/04/2021, 12:20:22 BST
Error: too much recursion
Source File: chrome://calendar/content/calendar-ui-utils.js
Line: 30

On inspection the code at this point is a bit confused, and can be improved by a patch like this:

--- a/lightning/chrome/calendar/content/calendar/calendar-ui-utils.js
+++ b/lightning/chrome/calendar/content/calendar/calendar-ui-utils.js
@@ -27,34 +27,34 @@
  *                        drop-downs
  */
 function setElementValue(aElement, aNewValue, aPropertyName) {
-    cal.ASSERT(aElement, "aElement");
-
-    if (aNewValue !== undefined) {
-        if (typeof aElement == "string") {
-            aElement = document.getElementById(aElement);
-            cal.ASSERT(aElement, "aElement");
-        }
-
-        if (!aElement) { return; }
-
+
+    if (aNewValue === undefined) { return; }
+    
+    if (typeof aElement == "string") {
+        aElement = document.getElementById(aElement);
+    }
+
+    if (!aElement) { return; }
+
+    if (aPropertyName) {
         if (aNewValue === false) {
             try {
                 aElement.removeAttribute(aPropertyName);
             } catch (e) {
                 cal.ERROR("setElementValue: aElement.removeAttribute couldn't remove " +
-                aPropertyName + " from " + (aElement && aElement.localName) + " e: " + e + "\n");
-            }
-        } else if (aPropertyName) {
+                aPropertyName + " from " + aElement.localName + " e: " + e + "\n");
+            }
+        } else {
             try {
                 aElement.setAttribute(aPropertyName, aNewValue);
             } catch (e) {
                 cal.ERROR("setElementValue: aElement.setAttribute couldn't set " +
-                aPropertyName + " from " + (aElement && aElement.localName) + " to " + aNewValue +
+                aPropertyName + " from " + aElement.localName + " to " + aNewValue +
                 " e: " + e + "\n");
             }
-        } else {
-            aElement.value = aNewValue;
-        }
+       }
+    } else {
+        aElement.value = aNewValue;
     }
 }
 

However with this patch the same error appears elsewhere:

Timestamp: 02/04/2021, 14:40:38 BST
Error: too much recursion
Source File: chrome://lightning/content/imip-bar.js
Line: 107

Possibly related, this error when navigating to a local mail folder:

Timestamp: 02/04/2021, 14:58:21 BST
Error: TypeError: browsers[i] is undefined
Source File: chrome://navigator/content/tabbrowser.xml
Line: 2248

But this looks like an issue from l.2346 where it is assumed that every tag has a defined linkedBrowser attribute, which must be untrue for the above message to be shown (does a mail tab have this?).

Expected results:

Not that!

Comment 1

[Frank-Rainer Grahl (:frg)]

Source File: chrome://lightning/content/imip-bar.js

I have seen these errors before but that was long ago. Do you have by chance an old user installed copy in your profile. Check if you have a remove button for Lightning and use it then. This should then install the official one during the next restart.

Error: TypeError: browsers[i] is undefined
Old error and unrelated. One of these days I will take a look.

Comment 2

[fieldhouse]

Thanks for the comment.

Do you have by chance an old user installed copy in your profile?

No, and the behaviour changed when I installed the version patched as above from the 2.53.7/Lightning 5.8.7 over the original.

I suspect this is an old issue that is being revealed again by some extension but haven't yet done the donkey-work of verifying (1) in safe mode (2) with binary chop en/dis/abling of extensions.

The generic explanation for "Too much recursion" is get/set/ting an attribute from within its get/set/ter, but as this would be an obvious error it normally the get/set/ter has to be doing this through some chain of calls.

On the browsers[i] issue, ll.253-5 of tabbrowser.xml, where the linkedBrowser attribute is tested for truthiness, imply that the attribute can be 'not a browser', so either change ll.2246 ff.:

             for (var i = 0; i < browsers.length; ++i)
-              if (browsers[i].contentWindow == aWindow)
+              # if browser is set, assume it is one
+              if (browsers[i] && (browsers[i].contentWindow == aWindow))
                 return this.tabs[i];

Or perhaps it could be removed at source with the change of l.2346:

-                  (this._browsers = Array.from(this.tabs, tab => tab.linkedBrowser));
+                  # only include linkedBrowsers when set
+                  (this._browsers = Array.from(this.tabs, tab => tab.linkedBrowser)).filter(browser => browser);