Last Comment Bug 603101 - Error: id is null Source file: chrome://.../addressingWidgetOverlay.js Line: 1038
: Error: id is null Source file: chrome://.../addressingWidgetOverlay.js Line: ...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Composition (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Philip Chee
:
Mentors:
Depends on: 256049
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-09 10:29 PDT by Philip Chee
Modified: 2010-10-17 21:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed fix v1.0 (1.23 KB, patch)
2010-10-09 10:44 PDT, Philip Chee
no flags Details | Diff | Splinter Review
Patch v1.1 use the .id like Thunderbird (1.20 KB, patch)
2010-10-10 08:48 PDT, Philip Chee
no flags Details | Diff | Splinter Review
Patch v1.2 use awMenulistKeyPress directly (3.64 KB, patch)
2010-10-15 08:18 PDT, Philip Chee
mnyromyr: review+
neil: superreview+
Details | Diff | Splinter Review

Description Philip Chee 2010-10-09 10:29:21 PDT
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 2010-10-09 10:44:53 PDT
Created attachment 482058 [details] [diff] [review]
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 2010-10-09 10:53:57 PDT
Comment on attachment 482058 [details] [diff] [review]
Proposed fix v1.0

># HG changeset patch
># User Philip Chee <philip.chee@gmail.com>
># 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 2010-10-10 08:48:29 PDT
Created attachment 482142 [details] [diff] [review]
Patch v1.1 use the .id like Thunderbird

Alternative patch.
Comment 4 Philip Chee 2010-10-10 08:49:16 PDT
Comment on attachment 482142 [details] [diff] [review]
Patch v1.1 use the .id like Thunderbird

># HG changeset patch
># User Philip Chee <philip.chee@gmail.com>
># 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 neil@parkwaycc.co.uk 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.
Comment 6 Philip Chee 2010-10-11 07:59:03 PDT
> 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 2010-10-11 10:41:52 PDT
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 2010-10-15 08:18:29 PDT
Created attachment 483482 [details] [diff] [review]
Patch v1.2 use awMenulistKeyPress directly

> neil@parkwaycc.co.uk      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 2010-10-17 21:22:43 PDT
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/db0b3e35e5b3

Note You need to log in before you can comment on or make changes to this bug.