Closed Bug 1263332 Opened 4 years ago Closed 4 years ago

Avoid startup JavaScript strict warning in DirectoryLinksProvider.jsm

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file)

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().
Attached patch patchSplinter Review
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+
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #2)
> Stealing this too since it's simple.

Thank you!
https://hg.mozilla.org/mozilla-central/rev/bd74613c5184
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.