Closed Bug 1278968 Opened 4 years ago Closed 4 years ago

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

Categories

(MailNews Core :: Address Book, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: aceman, Assigned: aceman)

Details

Attachments

(1 file)

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.
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.
(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 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+
Ok, thanks,
https://hg.mozilla.org/comm-central/rev/7f8a655fe06995a5d196dca3f21843d2791727a2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.