Closed Bug 1186346 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox Graveyard :: Reading List, defect)

defect
Not set
normal

Tracking

(firefox42 affected, firefox43 fixed)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox42 --- affected
firefox43 --- fixed

People

(Reporter: jruderman, Assigned: gioyik, Mentored)

Details

(Keywords: regression, testcase, Whiteboard: [good first bug][lang=js])

Attachments

(2 files, 1 obsolete file)

Attached 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]
Attached 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.
Attached 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+
I changed the commit message slightly.

remote:   https://hg.mozilla.org/integration/fx-team/rev/6bc329425006
https://hg.mozilla.org/mozilla-central/rev/6bc329425006
Status: NEW → RESOLVED
Closed: 5 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.