Last Comment Bug 753709 - JS error when pressing space on a Chat or Gloda facet view tab
: JS error when pressing space on a Chat or Gloda facet view tab
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 15.0
Assigned To: Florian Quèze [:florian] [:flo]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-10 04:24 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-05-15 03:20 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Possible patch (1.03 KB, patch)
2012-05-10 04:24 PDT, Florian Quèze [:florian] [:flo]
bwinton: review-
Details | Diff | Review
Patch v2 (simple null check) (920 bytes, patch)
2012-05-15 03:04 PDT, Florian Quèze [:florian] [:flo]
florian: review+
Details | Diff | Review

Description Florian Quèze [:florian] [:flo] 2012-05-10 04:24:30 PDT
Created attachment 622681 [details] [diff] [review]
Possible patch

JavaScript error: chrome://messenger/content/mailWindowOverlay.js, line 2368: contentWindow is null

This JS error happens at least:
- on the Chat tab (when no textbox has the focus)
- on the Gloda facet view tab
- on chromeTab (which is a bit surprising, given that the method works for contentTab).

The attached patch returns early if the currently selected tab is neither a mail tab nor a contentTab.
I'm not sure it's a good solution, but it does fix the noisy JS error.
Other possible approaches:
- just add a null check on the contentWindow variable
- attempt to rewrite the whole function in a way that would make more sense; maybe using the getBrowser method of tabs (I haven't tried this because I'm not sure I fully understand the current code; it took me a while to understand what the _mailrssiframe stuff was, it seemed like dead code at first).
Comment 1 Blake Winton (:bwinton) (:☕️) 2012-05-14 14:24:09 PDT
Comment on attachment 622681 [details] [diff] [review]
Possible patch

I think I would prefer a contentWindow null check, so that we can add other different tab types in future (one of which mconley might be adding right now)…

So, r-, but given the simplicity, you can get an automatic r=me if you implement that version.  ;)

Thanks,
Blake.
Comment 2 Florian Quèze [:florian] [:flo] 2012-05-15 03:04:38 PDT
Created attachment 623986 [details] [diff] [review]
Patch v2 (simple null check)

Carrying forward Blake's "automatic r=me if you implement that version".
Comment 3 Florian Quèze [:florian] [:flo] 2012-05-15 03:20:11 PDT
http://hg.mozilla.org/comm-central/rev/ff9e90e9f0d0

Note You need to log in before you can comment on or make changes to this bug.