Closed
Bug 1186346
Opened 9 years ago
Closed 9 years ago
JavaScript error: chrome://browser/content/tab-content.js, line 289: TypeError: content is null
Categories
(Firefox Graveyard :: Reading List, defect)
Firefox Graveyard
Reading List
Tracking
(firefox42 affected, firefox43 fixed)
RESOLVED
FIXED
Firefox 43
People
(Reporter: jruderman, Assigned: gioyik, Mentored)
Details
(Keywords: regression, testcase, Whiteboard: [good first bug][lang=js])
Attachments
(2 files, 1 obsolete file)
380 bytes,
text/html
|
Details | |
872 bytes,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
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•9 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•9 years ago
|
||
Attachment #8638247 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 3•9 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•9 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 5•9 years ago
|
||
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•9 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•9 years ago
|
||
Attachment #8638247 -
Attachment is obsolete: true
Attachment #8645537 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 8•9 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•9 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•9 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
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•