795978 - Error: this._containers is undefined in MarkupView.jsm Line: 327

Status: RESOLVED DUPLICATE of bug 852996

(DevTools :: Inspector, defect, P2)

Description

[Girish Sharma [:Optimizer]]

After opening and closing the html panel of inspector once, using the latest Nightly build of 1st October, I am getting these errors in my error console:

Timestamp: 10/1/2012 10:40:51 PM
Error: TypeError: this._containers is undefined
Source File: resource:///modules/devtools/MarkupView.jsm
Line: 327

Some mutation observer was not removed upon closing markup view.

Comment 1

[Rob Campbell [:rc] (:robcee)]

Noticed this today while looking for another problem with the Inspector.

I am able to get a bunch of these errors on the Web Console after I close the Inspector so it does seem like we have an observer sticking around.

Comment 2

[Paul Rouget [:paul]]

Dave, could you take a quick look at that?

Comment 4

[fms]

I found when I do a 
$.get(url, function(data){ 
   $('#mydiv').html(data);
}
and inside this "data" I have more jquery functions, the error occurs.

/**
   * Mutation observer used for included nodes.
   */
  _mutationObserver: function MT__mutationObserver(aMutations)
  {
    for (let mutation of aMutations) {

### here!!!!! this._containers is undefined
      let container = this._containers.get(mutation.target);
###
      if (!container) {
        // Container might not exist if this came from a load event for an iframe
        // we're not viewing.
        continue;
      }
      if (mutation.type === "attributes" || mutation.type === "characterData") {
        container.update();
      } else if (mutation.type === "childList") {
        this._updateChildren(container);
      }
    }
    this._inspector._emit("markupmutation");
  },

Comment 5

[Dave Camp (:dcamp)]

importNode is adding a load handler on the root that isn't being removed.

Comment 6

[Paul Rouget [:paul]]

https://mxr.mozilla.org/mozilla-central/source/browser/devtools/markupview/MarkupView.jsm#304

We need to save a reference to MP_watch_contentLoaded and remove it in the destroy function (MarkupView.jsm#625).

Comment 7

[Andreas Goetz]

I'm seubg a very similar error after upgradeing to 21 on the beta channel:
TypeError: this._containers is undefined @ resource://app/modules/devtools/MarkupView.jsm:330

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Patch v1

This removes the handler on destroy, which prevents the error from being triggered.

I wasn't quite sure how to test this change, but let me know if you have a suggestion.  I've verified that the existing markup view tests pass locally.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 744200 [details] [diff] [review]
Patch v1

Review of attachment 744200 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect, r+

Comment 10

[Rob Campbell [:rc] (:robcee)]

can this land?

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Yes, it's ready to land.

Comment 12

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/70538913a42e

cha cha!

Thanks Ryan. :)

Comment 13

[Rob Campbell [:rc] (:robcee)]

backed out:
https://hg.mozilla.org/integration/fx-team/rev/630e6f187cb2

Comment 14

[Paul Rouget [:paul]]

What's the status of this bug?

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I attempted a fix a few months back, but it led to further issues and was backed out.

I haven't had time to try again myself, so that's why I unassigned this one.

IIRC, my patch fixed the reported error, but then introduced intermittent testing issues.  For example, with the patch, markup view tests would pass when run independently, but not when run as a group.

Comment 16

[Brandon Benvie [:benvie]]

Actually, marking as P2 because this is a verified bug with a verified (but flawed) fix. We should get this fixed and relanded ASAP.

Comment 17

[christian.lange.81]

any progress on this?

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It looks like a similar fix to my attempted patch was made in bug 852996, so this should be taken care of now.