Last Comment Bug 784198 - browserIter.docShell is undefined errors
: browserIter.docShell is undefined errors
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 17
Assigned To: Jared Wein [:jaws] (please needinfo? me)
: Shane Caraveo (:mixedpuppy)
Depends on:
  Show dependency treegraph
Reported: 2012-08-20 16:14 PDT by :Gavin Sharp [email:]
Modified: 2012-08-23 03:47 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (2.24 KB, patch)
2012-08-21 14:48 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch (part 2) (6.74 KB, patch)
2012-08-21 14:50 PDT, Jared Wein [:jaws] (please needinfo? me)
jaws: review+
Details | Diff | Splinter Review
Patch v2 (part 1) (2.11 KB, patch)
2012-08-21 15:09 PDT, Jared Wein [:jaws] (please needinfo? me)
felipc: review+
Details | Diff | Splinter Review

Description User image :Gavin Sharp [email:] 2012-08-20 16:14:19 PDT
These are showing up in tests and sometimes in normal use as well. I think it's caused by us trying to access the docshell too shortly after document insertion of the iframe.
Comment 1 User image Jared Wein [:jaws] (please needinfo? me) 2012-08-21 14:48:01 PDT
Created attachment 653965 [details] [diff] [review]

This sets isAppTab to true when the panel is opened.

I tried adding a separate "load" event listener within updateButton but I still got iframe.docShell undefined errors (not sure why though). This patch works though without generating the errors.
Comment 2 User image Jared Wein [:jaws] (please needinfo? me) 2012-08-21 14:50:10 PDT
Created attachment 653966 [details] [diff] [review]
Patch (part 2)

This patch renames the variables from notifBrowser to notificationFrame (and related changes). Just some cleanup that Gavin did while looking at this bug.

r=me, author=gavin
Comment 3 User image :Felipe Gomes (needinfo me!) 2012-08-21 14:58:13 PDT
Comment on attachment 653965 [details] [diff] [review]

Review of attachment 653965 [details] [diff] [review]:

::: browser/base/content/browser-social.js
@@ +446,5 @@
>      });
>      panel.addEventListener("popupshown", function onpopupshown() {
>        panel.removeEventListener("popupshown", onpopupshown);
>        SocialToolbar.button.setAttribute("open", "true");

if notifBrowser.contentDocument exists, notifBrowser.docShell should exist as well. So you could set it unconditionally outside the if. But perhaps you have tried it and it didn't work?
Comment 4 User image Jared Wein [:jaws] (please needinfo? me) 2012-08-21 15:09:29 PDT
Created attachment 653977 [details] [diff] [review]
Patch v2 (part 1)

Thanks, I just didn't think of that. I tested it and it works fine.

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