dojox.mobile._ContentPaneMixin throws exception

[Bill Reed]

With Maqetta.org we allow users to dynamically change mobile themes within the html visual editor. When the theme is changed on a page we reparse the dojo widgets, the re-parsing of a dojox.mobile.ContentPane? with a href attribute throws and exception. dojo/parser::parse() error DOMException

parser.js:411

(anonymous function) parser.js:411 _4b8 dojo.js:15 _4b7.then._4c5.then dojo.js:15 lang.extend.otherwise dojo.js:15 _1a.parse parser.js:410 _2246._processWidgets davinci.js:48614 dojo.ready.dojo.addOnLoad._5de dojo.js:15 _5da dojo.js:15 _5d6 dojo.js:15 dojo.ready.dojo.addOnLoad dojo.js:15 _2246._processWidgets davinci.js:48612 _447 dojo.js:5518 _446.then._454.then dojo.js:5628 _2246._processWidgets davinci.js:48611 _2246._setSourceData davinci.js:48587 _2246._continueLoading davinci.js:48464 _2246._setSource davinci.js:48357 ret.withDoc dojo.js:6549 _2246.setSource davinci.js:48207

I have debug the problem dojox.mobile.ProgressIndicator

Here is the flow of what happens When a dojox.mobile._ContentPaneMixin is constructed a instance of a progress indicator is retrieved

constructor: function(){
            // summary:
            //      Creates a new instance of the class.
            // tags:
            //      private
            if(this.prog){
                this._p = ProgressIndicator.getInstance();
            }
        },

The ProgressIndicator.getInstance creates a ProgressIndicator if one does not already exist and assigned it to the global cls._instance

cls.getInstance = function(props){
        if(!cls._instance){
            cls._instance = new cls(props);
        }
        return cls._instance;
    };

dojox.mobile._ContentPaneMixin._setHrefAttr is callled if we have an instences of ProgressIndicator it's dom is appended to the body

            if(this.lazy || !href || href === this._loaded){
                this.lazy = false;
                return null;
            }
            var p = this._p;
            if(p){
                win.body().appendChild(p.domNode);
                p.start();
            }

Once the content is loaded from the href dojox.mobile._ContentPaneMixin._setContentAttr is called which then calls the ProgressIndicator.stop()

        
            this.destroyDescendants();
            if(typeof data === "object"){
                this.containerNode.appendChild(data);
            }else{
                if(this.executeScripts){
                    data = this.execScript(data);
                }
                this.containerNode.innerHTML = data;
            }
            if(this.parseOnLoad){
                var _this = this;
                return Deferred.when(lazyLoadUtils.instantiateLazyWidgets(_this.containerNode), function(){
                    if(_this._p){ _this._p.stop(); }
                    return _this.onLoad();
                });
            }
            if(this._p){ this._p.stop(); }
            return this.onLoad();
        }

in ProgressIndicator.stop the domNode of the progress indicator is removed, but the cls._instance in not set to null

stop: function(){
            // summary:
            //      Stops the spinning of the ProgressIndicator.
            if(this.timer){
                clearInterval(this.timer);
            }
            this.timer = null;
            if(this.removeOnStop && this.domNode && this.domNode.parentNode){
                this.domNode.parentNode.removeChild(this.domNode);
            }
        },

So that the next time the _ContentPaneMixin constructor is call the ProgressIndicator.getInstance(); returns the old instance of ProgressIndecator that is missing the domNode. So that when dojox.mobile._ContentPaneMixin._setHrefAttr and attempts to execute the line

var p = this._p;
            if(p){
                win.body().appendChild(p.domNode);
                p.start();
            }

an exception is thrown

if I add code to dojox.mobile.ProgressIndicator.stop that clears the cls._instance

stop: function(){
            // summary:
            //      Stops the spinning of the ProgressIndicator.
            if(this.timer){
                clearInterval(this.timer);
            }
            this.timer = null;
            if(this.removeOnStop && this.domNode && this.domNode.parentNode){
                this.domNode.parentNode.removeChild(this.domNode);
                cls._instance = null; // wdr
            }
        },

then the exception is resolved.

[Bill Reed]

Possible patch

[Bill Reed]

I have attached a possible patch, Also here is a link to the Maqetta github issue ​https://github.com/maqetta/maqetta/issues/3394

[Eric Durocher]

I don't really see a problem in this code sequence.

The stop method remove the ProgressIndicator's domNode:

                this.domNode.parentNode.removeChild(this.domNode);

Then _ContentPaneMixin re-adds it to the document:

                win.body().appendChild(p.domNode);

That seems correct to me. Also I don't quite understand this:

> ProgressIndicator.getInstance(); returns the old instance of ProgressIndicator that is missing the domNode

What do you mean by "missing"? Is the domNode reference null? It has been removed from its parent, but it should still be a valid node that can be added again to the DOM.

Are you sure that there no other code that would accidentally destroy the ProgressIndicator?

Can you reproduce on a simple sample, outside of Maqetta?

[Bill Reed]

Replying to edurocher:

I don't really see a problem in this code sequence.

The stop method remove the ProgressIndicator's domNode:

                this.domNode.parentNode.removeChild(this.domNode);

Then _ContentPaneMixin re-adds it to the document:

                win.body().appendChild(p.domNode);

That seems correct to me. Also I don't quite understand this:

> ProgressIndicator.getInstance(); returns the old instance of ProgressIndicator that is missing the domNode

What do you mean by "missing"? Is the domNode reference null? It has been removed from its parent, but it should still be a valid node that can be added again to the DOM.

Correct the ProgressIndicator?.getInstrance() returns the ProgressIndicator? object p the object does not have a domNode property, p.domNode must have been delete somewhere.

Are you sure that there no other code that would accidentally destroy the ProgressIndicator?

We do not have any code that would delete the progress indicator. We do destroy and recreate the ContentPane?

Can you reproduce on a simple sample, outside of Maqetta?

I have not tried to recreate outside of Maqetta. I was hoping you might have an idea where the domNode is being destroyed. I will attempt to recreate outside of maqetta.

[Eric Durocher]

Thanks. No, sorry I don't see any call (at least in dojox/mobile) that could possibly destroy the indicator (especially since it is added directly to the body, so it could not be destroyed as part of a destroyDescendants for example).

[Bill Reed]

We do destroy all the dojo widgets in the registry before we re-parse the page so we do not get widgetid already defined errors. It looks like when the ContentPane? or the progress bar are destoryed they do not delete the cls._instance

Here is an HTML test file that reproduces the error

<!DOCTYPE html>
<html>
<head>
	<meta http-equiv="Content-type" content="text/html; charset=utf-8"/>
	<meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,minimum-scale=1,user-scalable=no"/>
	<meta name="apple-mobile-web-app-capable" content="yes"/>
	<title>ContentPane</title>

	<script type="text/javascript" src="../deviceTheme.js" data-dojo-config="mblThemeFiles: ['base']"></script>
	<script type="text/javascript" src="../../../dojo/dojo.js" data-dojo-config="async: true, parseOnLoad: true"></script>

	<script type="text/javascript">
		require([
			"dojo/dom-construct",
			"dojo/ready",
			"dijit/registry",
			"dojox/mobile/parser",
			"dojox/mobile",
			"dojox/mobile/compat",
			"dojox/mobile/ContentPane"
		], function(domConstruct, ready, registry, parser, mobile,compat,ContentPane){
			ready(function(){
				window.setTimeout(function(){
					dijit.registry.toArray().forEach(function(w){
                 		w.destroy();             
         		 	});
					var cp = new ContentPane({id:"pane2", href:"data/fragment2.html"},'div1');
				},2000);
			});
		});
	</script>
</head>
<body style="visibility:hidden;">
 <div id='div1'>
		<div id="pane2" data-dojo-type="dojox.mobile.ContentPane"
		     href="data/fragment1.html"></div>


	</div>
</div>
</body>
</html>

[Bill Reed]

A better fix to to add a destroy() method to ProgressIndicator

I still need to test this fix inside Maqetta but I bet it will work

[Bill Reed]

Better path, add destroy method

[Bill Reed]

I tested the above patch and it does fix the issue on Maqetta

[Eric Durocher]

Definitely better yes, but there is no reason to clear the singleton if another ProgressIndicator instance is destroyed, so we should test that the destroyed instance is actually the shared instance. I am attaching a modified patch, could you also test this in Maqetta please? thanks.

[Eric Durocher]

Test if the destroyed instance is the shared instance - Eric Durocher (IBM, CCLA)

[Bill Reed]

Replying to edurocher:

Definitely better yes, but there is no reason to clear the singleton if another ProgressIndicator instance is destroyed, so we should test that the destroyed instance is actually the shared instance. I am attaching a modified patch, could you also test this in Maqetta please? thanks.

This fix works on Maqetta.

[Eric Durocher]

OK, thanks for the test, then for me this patch 3 is OK to commit.

[Bill Reed]

Replying to edurocher:

OK, thanks for the test, then for me this patch 3 is OK to commit.

It works for us.

[cjolif]

In [29704]:

refs #16002. Thanks Eric Durocher (IBM, CCLA).

[cjolif]

In [29705]:

fixes #16002. Thanks Eric Durocher (IBM, CCLA).