Avoid startup JavaScript strict warning in DirectoryLinksProvider.jsm

RESOLVED FIXED in Firefox 48

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

unspecified
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
During startup the console noise includes:

JavaScript strict warning: resource:///modules/DirectoryLinksProvider.jsm, line 647: ReferenceError: reference to undefined property link.enhancedImageURI

The |this.isURLAllowed(link.enhancedImageURI, ALLOWED_IMAGE_SCHEMES)| check added in http://hg.mozilla.org/mozilla-central/rev/c742dcb56135 (bug 1088729) is causing this. The comment there implies that |link.imageURI| may also be undefined.

The code here should check that link.imageURI and link.enhancedImageURI are defined before passing them to isURLAllowed().
(Assignee)

Comment 1

3 years ago
Created attachment 8740913 [details] [diff] [review]
patch
Assignee: nobody → jwatt
Attachment #8740913 - Flags: review?(edilee)
Comment on attachment 8740913 [details] [diff] [review]
patch

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

Stealing this too since it's simple.

::: browser/modules/DirectoryLinksProvider.jsm
@@ +644,5 @@
>          // Make sure the link url is allowed and images too if they exist
>          return this.isURLAllowed(link.url, ALLOWED_LINK_SCHEMES, false) &&
> +               (!('imageURI' in link) ||
> +                this.isURLAllowed(link.imageURI, ALLOWED_IMAGE_SCHEMES, checkBase)) &&
> +               (!('enhancedImageURI' in link) ||

This file and most in /browser/ uses double-quotes… but we generally prefer the simpler truthiness check instead of `in` which also avoids a pair of braces:
(!link.imageURI || this.isURLAllowed(link.imageURI, ALLOWED_IMAGE_SCHEMES, checkBase)) &&
(!link.enhancedImageURI || this.isURLAllowed(link.enhancedImageURI, ALLOWED_IMAGE_SCHEMES, checkBase));
Attachment #8740913 - Flags: review?(edilee) → review+
(Assignee)

Comment 3

3 years ago
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #2)
> Stealing this too since it's simple.

Thank you!

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bd74613c5184
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.