603101 - Error: id is null Source file: chrome://.../addressingWidgetOverlay.js Line: 1038

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, defect)

Description

[Philip Chee]

From Thunderbird Bug #256049:

> timeless      2004-08-18 13:17:20 PDT
> 
> Error ``id has no properties'' [xs] in file
> ``chrome://messenger/content/messengercompose/addressingWidgetOverlay.js'', line
> 1057, character 0.
> Stopped for error handler.
> #0: function awDocumentKeyPress(event=Event:{0}) in
> <chrome://messenger/content/messengercompose/addressingWidgetOverlay.js> line 1057
> 1055: try {
> 1056: var id = event.target.getAttribute('id');
> 1057: if (id.substr(0, 11) == 'addressCol1')
> 1058: awMenulistKeyPress(event, event.target);
> 1059: } catch (e) { }

On SeaMonkey trunk I get this:

Error: id is null
Source file: chrome://messenger/content/messengercompose/addressingWidgetOverlay.js
Line: 1038

Comment 1

[Philip Chee]

Attachment: Proposed fix v1.0

> -    if (id.substr(0, 11) == 'addressCol1')
> +    if (id && id.substr(0, 11) == 'addressCol1')

If we are in the content area we don't have any .id properties natch.

> -  } catch (e) { }
> +  }
> +  catch (e)

Do we still need a try catch block here?

Also we should consider moving the eventListener from the document to nearer the menulist as suggested by Neil (in Bug 124304 Comment 24). Now that the popup handling code has been rewritten by the other Neil the original problems probably don't exist any more.

Comment 2

[Philip Chee]

Comment on attachment 482058 [details] [diff] [review]
Proposed fix v1.0

># HG changeset patch
># User Philip Chee <[e-mail]>
># Date 1286645486 -28800
># Node ID edc064f9beb1ae8ae103747b5952aae1329a9e3e
># Parent  7661b2db37db5c2b06608be788243ff805c6412c
>Bug 603101 Error: id is null Source file: chrome://.../addressingWidgetOverlay.js Line: 1038
>
>diff --git a/suite/mailnews/compose/addressingWidgetOverlay.js b/suite/mailnews/compose/addressingWidgetOverlay.js
>--- a/suite/mailnews/compose/addressingWidgetOverlay.js
>+++ b/suite/mailnews/compose/addressingWidgetOverlay.js
>@@ -1028,21 +1028,26 @@ function awSizerResized(aSplitter)
>   listbox.height = listbox.boxObject.height;
>   // remove all the heights set on the splitter's previous siblings
>   for (let sib = aSplitter.previousSibling; sib; sib = sib.previousSibling)
>     sib.removeAttribute("height");
> }
> 
> function awDocumentKeyPress(event)
> {
>-  try {
>+  try
>+  {
>     var id = event.target.getAttribute('id');
>-    if (id.substr(0, 11) == 'addressCol1')
>+    if (id && id.substr(0, 11) == 'addressCol1')
>       awMenulistKeyPress(event, event.target);
>-  } catch (e) { }
>+  }
>+  catch (e)
>+  {
>+    Components.utils.reportError(e);
>+  }
> }
> 
> function awRecipientInputCommand(event, inputElement)
> {
>   gContentChanged=true; 
>   setupAutocomplete(); 
> }
>

Comment 3

[Philip Chee]

Attachment: Patch v1.1 use the .id like Thunderbird

Alternative patch.

Comment 4

[Philip Chee]

Comment on attachment 482142 [details] [diff] [review]
Patch v1.1 use the .id like Thunderbird

># HG changeset patch
># User Philip Chee <[e-mail]>
># Date 1286725449 -28800
># Node ID 2836f5570039bdad359da435cc83f1e7770320aa
># Parent  bfd1320b39acbd8c1aaab876abbd13196f2dfda3
>Bug 603101 Error: id is null Source file: chrome://.../addressingWidgetOverlay.js Line: 1038
>
>diff --git a/suite/mailnews/compose/addressingWidgetOverlay.js b/suite/mailnews/compose/addressingWidgetOverlay.js
>--- a/suite/mailnews/compose/addressingWidgetOverlay.js
>+++ b/suite/mailnews/compose/addressingWidgetOverlay.js
>@@ -1029,20 +1029,23 @@ function awSizerResized(aSplitter)
>   // remove all the heights set on the splitter's previous siblings
>   for (let sib = aSplitter.previousSibling; sib; sib = sib.previousSibling)
>     sib.removeAttribute("height");
> }
> 
> function awDocumentKeyPress(event)
> {
>   try {
>-    var id = event.target.getAttribute('id');
>-    if (id.substr(0, 11) == 'addressCol1')
>+    var id = event.target.id;
>+    if (id && id.substr(0, 11) == 'addressCol1')
>       awMenulistKeyPress(event, event.target);
>-  } catch (e) { }
>+  }
>+  catch (e) {
>+    Components.utils.reportError(e);
>+  }
> }
> 
> function awRecipientInputCommand(event, inputElement)
> {
>   gContentChanged=true; 
>   setupAutocomplete(); 
> }
>

Comment 5

[[e-mail]]

Ideally we would ditch awDocumentKeyPress and call awMenulistKeyPress directly from the menulist like we do with awRecipientKeyPress from the textbox.

Comment 6

[Philip Chee]

> Ideally we would ditch awDocumentKeyPress and call awMenulistKeyPress directly
> from the menulist like we do with awRecipientKeyPress from the textbox.
So if I use onkeypress on the menulist I don't have to use a capturing eventlistener as stated in Bug 124304 Comment 24?

Comment 7

[Philip Chee]

STR: start in the address widget. Tab into the content area type a bit tab a bit. Look at the error console.

Comment 8

[Philip Chee]

Attachment: Patch v1.2 use awMenulistKeyPress directly

> [e-mail]      2010-10-10 11:41:41 PDT
> 
> Ideally we would ditch awDocumentKeyPress and call awMenulistKeyPress directly
> from the menulist like we do with awRecipientKeyPress from the textbox.
This works 100%.
New patch using this approach.

Comment 9

[Philip Chee]

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/db0b3e35e5b3