Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#9548 closed defect (fixed)

problems attaching style sheet to dijit.Editor

Reported by: ben hockey Owned by:
Priority: high Milestone: 1.4
Component: Editor Version: 1.3.1
Keywords: Cc:
Blocked By: Blocking:

Description

this seems to only happen when creating the editor programmatically...

in dijit._editor.RichText?.js - the addStyleSheet function tries to reference this.document.createStyleSheet. because of the timeout that waits for the iframe to load in the ifrFunc in _drawIframe, the document might not be ready yet - so we get an error that this.document is undefined.

I have attached a test case (place as a sibling of dojo, dijit, dojox).

workaround is to put something like this in _initButton of the test case but that doesn't address the cause of this problem.

var dfd = dojo.connect(this.button, 'onFocus', this, function(){
	dojo.disconnect(dfd);
	this.editor.addStyleSheet(dojo.moduleUrl('dijit.themes', 'dijit.css'));
});

Attachments (1)

editor_bug.html (2.6 KB) - added by ben hockey 8 years ago.

Download all attachments as: .zip

Change History (6)

Changed 8 years ago by ben hockey

Attachment: editor_bug.html added

comment:1 Changed 8 years ago by bill

The setTimeout() calls are gone in trunk but it's likely that setEditor() is still getting called before the iframe has initialized.

I'm not sure what makes sense here though, it seems like some plugins need to be initialized early because they setup pre-filters on the data being edited... like the EnterKeyHandling plugin for example.

comment:2 in reply to:  1 ; Changed 8 years ago by ben hockey

Replying to bill:

I'm not sure what makes sense here though, it seems like some plugins need to be initialized early because they setup pre-filters on the data being edited... like the EnterKeyHandling plugin for example.

is it overkill to have all (or at least most) references to the document go through a callback of onLoadDeferred? i see that this is what _setDisableSpellCheckAttr is doing. to be honest, i haven't looked to see if it makes sense for all the code but since onLoadDeferred is analogous to dojo.addOnLoad then the safest way to ensure the document exists would be to only reference it through that callback.

for this specific case of adding a stylesheet i think it should work - not sure about other cases.

    this.onLoadDeferred.addCallback(dojo.hitch(this, function(){
		if(this.document.createStyleSheet){ //IE
			this.document.createStyleSheet(url);
		}else{ //other browser
			var head = this.document.getElementsByTagName("head")[0];
			var stylesheet = this.document.createElement("link");
			stylesheet.rel="stylesheet";
			stylesheet.type="text/css";
			stylesheet.href=url;
			head.appendChild(stylesheet);
		}
    }));

comment:3 in reply to:  2 Changed 8 years ago by ben hockey

Replying to neonstalwart:

is it overkill to have all (or at least most) references to the document go through a callback of onLoadDeferred? i see that this is what _setDisableSpellCheckAttr is doing. to be honest, i haven't looked to see if it makes sense for all the code but since onLoadDeferred is analogous to dojo.addOnLoad then the safest way to ensure the document exists would be to only reference it through that callback.

just briefly took a better look at the code and perhaps we would not want/need all (or even most) references to the document to go through the callback but it looks like this same issue could possibly arise in _setDisabledAttr but the references to the document are in try/catch blocks so it might never have revealed itself previously.

maybe i'll just stick to commenting on this particular case (adding a style sheet) and suggest that it might be a good fix for this case. :)

comment:4 Changed 8 years ago by ben hockey

Resolution: fixed
Status: newclosed

(In [18834]) changed code to add stylesheet via callback from onLoadDeferred. fixes #9548 !strict

comment:5 Changed 8 years ago by bill

Milestone: tbd1.4
Note: See TracTickets for help on using tickets.