1388882 - some JS warnings in mail/components/preferences/applications.js

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[:aceman]

In preferences -> Attachments -> Incoming select Content-type e.g. 'http', in Action choose "Application details...".

I get a dialog with empty list and these errors in the Error console:

23:11:54.740 ReferenceError: reference to undefined property "http" 1 applications.js:1153:1

23:11:54.745 ReferenceError: AppConstants is not defined 1 applications.js:1274:1
	_isValidHandlerExecutable chrome://messenger/content/preferences/applications.js:1274:1
	isValidHandlerApp chrome://messenger/content/preferences/applications.js:1256:14
	appManager_init chrome://messenger/content/preferences/applicationManager.js:24:12
	onload chrome://messenger/content/preferences/applicationManager.xul:1:1
	openSubDialog chrome://global/content/bindings/preferences.xml:1006:18
	manageApp chrome://messenger/content/preferences/applications.js:1599:7
	oncommand chrome://messenger/content/preferences/preferences.xul:1:1

Comment 1

[:aceman]

Attachment: patch

Adds the missing AppConstants.jsm include and convert _visibleTypeDescriptionCount to a Map so that queries for non-existent members do not produce a warning.

Comment 2

[:aceman]

After the patch, the dialog is no longer empty and shows "firefox" in the list.

The AppConstants error may be caused with bug 1278337, which landed in TB50.
So this may be an uplift candidate.

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 8895555 [details] [diff] [review]
patch

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

I had to find something I could comment on ;-)

::: mail/components/preferences/applications.js
@@ +1068,5 @@
>        this._visibleTypes.push(handlerInfo);
>  
> +      let descCount = 0;
> +      if (this._visibleTypeDescriptionCount.has(handlerInfo.description))
> +        descCount = this._visibleTypeDescriptionCount.get(handlerInfo.description);

Maybe nicer with ? : ;

You could write the hole thing in one line:
x.set(k, x.has(k) ? x.get(k) + 1 : 1);

Up to you, Mr. Elegant ;-)

Comment 4

[:aceman]

But when x is 'this._visibleTypeDescriptionCount' then how would it fit on a line? Wouldn't it be much uglier when the ?: operator is split into multiple lines?

Comment 5

[Jorg K (CEST = GMT+2)]

Yes, I know, maybe the first line could be:

let descCount = this._visibleTypeDescriptionCount.has(handlerInfo.description) ?
  this._visibleTypeDescriptionCount.get(handlerInfo.description) : 1;

That's OK, no?

Comment 6

[:aceman]

Yes, that is 1-2 lines shorter than my version :)

Comment 7

[:aceman]

Attachment: patch v1.1

Comment 8

[Jorg K (CEST = GMT+2)]

Thanks, I landed the unrelated hunks in the other bug.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d96ce51dc17a
fix warnings in Application details for attachment types. r=jorgk