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

RESOLVED FIXED in Firefox 43

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jruderman, Assigned: gioyik, Mentored)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
Firefox 43
regression, testcase

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8637067 [details]
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");
  },

Comment 1

3 years ago
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]
(Assignee)

Comment 2

3 years ago
Created attachment 8638247 [details] [diff] [review]
1186346.patch
Attachment #8638247 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 3

3 years ago
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 4

3 years ago
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+
(Assignee)

Comment 6

3 years ago
I forgot this issue due I was not assigned and didn't appear in my dashboard. Attaching a new patch.
(Assignee)

Comment 7

3 years ago
Created attachment 8645537 [details] [diff] [review]
1186346.patch
Attachment #8638247 - Attachment is obsolete: true
Attachment #8645537 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 8

3 years ago
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 9

3 years ago
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+

Comment 10

3 years ago
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
Last Resolved: 3 years ago
status-firefox43: --- → fixed
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.