If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

multiple "TypeError: params is null" in ab*AutoComplete*.js when running tests.

RESOLVED FIXED in Thunderbird 50.0

Status

MailNews Core
Address Book
--
minor
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 50.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

5.79 KB, patch
Magnus Melin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

a year ago
Some tests produce "TypeError: params is null" JS errors.

Can be seen e.g. in: http://archive.mozilla.org/pub/thunderbird/nightly/2016/06/2016-06-08-03-02-08-comm-central/comm-central_xp_ix_test-mozmill-bm112-tests1-windows-build26.txt.gz

04:39:20     INFO -  TEST-START | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test_mark_messages_forwarded
04:39:20     INFO -  JavaScript error: resource://gre/components/nsAbAutoCompleteMyDomain.js, line 22: TypeError: params is null
04:39:20     INFO -  JavaScript error: resource://gre/components/nsAbAutoCompleteSearch.js, line 338: TypeError: params is null
04:39:20     INFO -  JavaScript error: resource://gre/components/nsAbLDAPAutoCompleteSearch.js, line 171: TypeError: params is null
04:39:20     INFO -  JavaScript error: resource://gre/components/nsNewsAutoCompleteSearch.js, line 99: TypeError: params is null
04:39:22     INFO -  JavaScript error: resource://gre/components/nsAbAutoCompleteMyDomain.js, line 22: TypeError: params is null
04:39:22     INFO -  JavaScript error: resource://gre/components/nsAbAutoCompleteSearch.js, line 338: TypeError: params is null
04:39:22     INFO -  JavaScript error: resource://gre/components/nsAbLDAPAutoCompleteSearch.js, line 171: TypeError: params is null
04:39:22     INFO -  JavaScript error: resource://gre/components/nsNewsAutoCompleteSearch.js, line 99: TypeError: params is null
04:39:30     INFO -  TEST-PASS | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test-message-commands-on-msgstore.js::test_mark_messages_forwarded

I've tracked it down to tests that type the recipient in compose and then close the window. The autocomplete has no time to kick in and the params argument is null. Adding some sleep time fixed the problem for me (no more error). Alternatively we can accept empty params.
(Assignee)

Comment 1

a year ago
Created attachment 8761379 [details] [diff] [review]
patch

Try run: https://hg.mozilla.org/try-comm-central/rev/4c89fd6d8e503653fdf2e5c7b09509f9be2fa61a
Attachment #8761379 - Flags: review?(mkmelin+mozilla)

Comment 2

a year ago
Comment on attachment 8761379 [details] [diff] [review]
patch

Review of attachment 8761379 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/addrbook/src/nsAbLDAPAutoCompleteSearch.js
@@ +188,5 @@
>      // use that, otherwise, try the global preference instead.
>      var acDirURI = null;
>      var identity;
>  
> +    if ("idKey" in params) {

Drive-by question for my own education [learning on the job ;-)]:
Is this equivalent? Can "params" not have a property "idKey", yet the value is null or ""? (multiple occasions).
Otherwise this patch looks good to me.
(Assignee)

Comment 3

a year ago
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #2)
> Drive-by question for my own education [learning on the job ;-)]:
> Is this equivalent? Can "params" not have a property "idKey", yet the value
> is null or ""? (multiple occasions).

Yes, if there is no idKey property in params and you ask for params.idKey it returns the special value undefined (or similar). However in gecko JS, ot also throws a warning that you are accessing nonexistent property. On the other hand, it is common pattern to do "if (params.idKey)" to test if the property exists and in this single case I think there is no warning. Anyway, ("idKey" in params) should be clean an warning free, as it is a valid expression and returns true or false, so it does not rely on undefined being handled as false and hoping there is no warning :) Of course params must be an object and not null.

Comment 4

a year ago
Comment on attachment 8761379 [details] [diff] [review]
patch

Review of attachment 8761379 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin with the below changed at all the places

::: mailnews/addrbook/src/nsAbAutoCompleteMyDomain.js
@@ +17,5 @@
>  
>    applicableHeaders: new Set(["addr_to", "addr_cc", "addr_bcc", "addr_reply"]),
>  
>    startSearch: function(aString, aSearchParam, aResult, aListener) {
> +    let params = JSON.parse(aSearchParam) || {};

Not sure I like this much as it kind of looks like "try parse it, if unsuccessful just assign to {}". Well it only works if aSearchParam is null, not "" or undefined etc. 

Maybe better to have it

  let params = aSearchParam ? JSON.parse(aSearchParam) : {};
Attachment #8761379 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 5

a year ago
Ok, thanks,
https://hg.mozilla.org/comm-central/rev/7f8a655fe06995a5d196dca3f21843d2791727a2
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.