JavaScript error: chrome://browser/content/tab-content.js, line 289: TypeError: content is null

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jruderman, Assigned: gioyik, Mentored)

Tracking

(Blocks 1 bug, {regression, testcase})

Trunk
Firefox 43
Dependency tree / graph

Firefox Tracking Flags

(firefox42 affected, firefox43 fixed)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(2 attachments, 1 obsolete attachment)

Posted file testcase
JavaScript error: chrome://browser/content/tab-content.js, line 289: TypeError: content is null

This code was added in https://hg.mozilla.org/mozilla-central/rev/ad3867772421:

  get isAboutReader() {
    return content.document.documentURI.startsWith("about:reader");
  },
I think it would be safe enough to add a null check here and just return false if content is null.

To fix this bug, you need to update the code here:
http://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/browser/base/content/tab-content.js#l288
Mentor: margaret.leibovic
Whiteboard: [good first bug][lang=js]
Posted patch 1186346.patch (obsolete) — Splinter Review
Attachment #8638247 - Flags: review?(margaret.leibovic)
Hi Margaret,

I created a patch for this one, let me know if the null check is correct and the patch is a good fix for this.

Thanks
Comment on attachment 8638247 [details] [diff] [review]
1186346.patch

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

Thanks for the patch! This looks reasonable to me, but I'd like a second opinion from a /browser peer, since I'm mainly a mobile developer :)
Attachment #8638247 - Flags: review?(margaret.leibovic)
Attachment #8638247 - Flags: review?(jaws)
Attachment #8638247 - Flags: review+
Comment on attachment 8638247 [details] [diff] [review]
1186346.patch

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

::: browser/base/content/tab-content.js
@@ +287,5 @@
>  
>    get isAboutReader() {
> +    if (content == null) {
> +      return false;
> +    }

This should just be:
if (!content) {
  return false;
}
Attachment #8638247 - Flags: review?(jaws) → review+
I forgot this issue due I was not assigned and didn't appear in my dashboard. Attaching a new patch.
Posted patch 1186346.patchSplinter Review
Attachment #8638247 - Attachment is obsolete: true
Attachment #8645537 - Flags: review?(margaret.leibovic)
Not setting review? to Jared due seems he is PTO.

If this looks good, I could push it to try server, I only need the try syntax.

Thanks
Assignee: nobody → gioyik
Comment on attachment 8645537 [details] [diff] [review]
1186346.patch

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

Nice! I don't think this needs a try-push, will land it for you in a second.
Attachment #8645537 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/6bc329425006
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.