Opened 5 years ago

Closed 5 years ago

#17847 closed defect (fixed)

Editor loses its value when moved around.

Reported by: jandockxppw Owned by: bill
Priority: undecided Milestone: 1.10
Component: Editor Version: 1.9.3
Keywords: Cc:
Blocked By: Blocking:

Description

http://jsfiddle.net/jandockx/mgv7a/ demonstrates the problem.

This is part of a wider issue, which is more difficult to demonstrate with a build version.

The heart of the matter is, that when the editor is moved around in the dom (e.g., using domConstruct.place on a widget that contains the editor some levels deep, as in our case), at least on Chrome, the iframe inside loses its contents, and will be "reloaded", i.e., reinitialized.

The fiddle demonstrates that this happens.

The direct problem the fiddle demonstrates is that when used this way, the Editor doesn't retain its value. The value is reset to the initial value on move. The main reason for this is that the "html" parameter of onLoad is bound in a Closure in open for its entire lifetime. I believe the actual "content" of the iframe should be this.get("value") at the latest time possible.

There is no amount of binding that can resolve this issue, since onLoad does call setValue. It obviously shouldn't: that is not the task of onLoad.

We resolved this in a subclass, by adding a "modelValue" property we bind to, and overriding onLoad:

 onLoad: function(html) {
        /* This method is called in RichText with a html based on a number of things, like this.value, the contents
           of the tag, etc. We want the modelValue to be authoritative, but amount of initialization helps.
           Therefor, we just kidnap the call, and inject the html we want. */
        var superMethod = this.getInherited(arguments);
        var actualHtml = format(this.get("modelValue"));
        logger.debug("changed parameter of onLoad from '" + html + "' to '" + actualHtml + "'");
        return superMethod.call(this, actualHtml);
      }

The wider problem implies that also onLoadDeferred is resolved again and again. An error is logged for this when you are not using a build version. In any case, the deferred is not really "resolved again", so any other "initialization code" that is written in a "then" is not executed again. Editor should provide a mechanism for correct "reinitialization" in all cases. There is quite a lot of dijit code that uses onLoadDeferred.

Change History (4)

comment:1 Changed 5 years ago by bill

Component: DijitEditor

I'm sure that there's a reason that onLoad calls setValue(); probably it's because encoding the actual editor value in the iframe's javascript: URL will exceed some length limit (for editors with large initial values, and on certain browsers).

Your suggestion though that onLoad sets the latest value rather than the initial value seems reasonable. It might also solve a race condition if the app set the value while the iframe was loading?

About onLoadDeferred getting resolved repeatedly, as you said it just produces spurious warnings, and is not a real issue. Unless I missed something? Still, it would be nice to fix.

comment:2 Changed 5 years ago by bill

Milestone: tbd1.10
Owner: set to bill
Status: newassigned

comment:3 Changed 5 years ago by bill

Anyway, thanks for tracking down the problem to the multiple calls to onLoad(), and the closure giving the stale value (i.e. the html variable). I'll check in a fix now, although there may be other bugs lurking.

Note that moving the iframe actually clears its value, so we can't simply ignore repeated calls to iframe.onload. We have to repeat setting the iframe's value. There's probably some stuff we are repeating unnecessarily though (when the iframe is moved).

comment:4 Changed 5 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In 9834dcbe9080624f0f73d0772937818c5057d8b4/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.