moinejf/abc2svg

abc2svg.modules.load - definition refers to user but it might not be defined

Closed this issue · 10 comments

bwl21 commented

abc2svg.modules.load passes a function to loadjs which refers to user which is only available in an instance of abc2svg.Abc.

				abc2svg.loadjs(m.fn,
				    function() {	// if success
					if (--abc2svg.modules.nreq == 0)
						abc2svg.modules.cbf()
				    },
				    function() {	// if error
					user.errmsg('error loading ' + m.fn);
					if (--abc2svg.modules.nreq == 0)
						abc2svg.modules.cbf()
				    })

It is absolutely reasonable (I even had proposed this) to apply user.errmsg here. But I guess it will be necessary to still pass the current instance of abc2svg.Abc as an argument to abc2svg.modules.load as it was before d6d4cd5

Maybe it would even be better if abc2.svg.moudules.load had three arguments:

  • ABC_source (source)
  • ondone (function)
  • onerror (function) this function should get one argument (the name of the module which could not be loaded)
bwl21 commented

I am playing around with commit 43dc562. I removed "MIDI-1.js", but do not provide onerror (just for testing). I get the message on the console

abc2svg-1.self.js?body=1:20427 Uncaught TypeError: Illegal invocation
    at HTMLScriptElement.<anonymous> (abc2svg-1.self.js?body=1:20427)

The respective line is module.js:92. It works if I change this line to (But I do not know the reason)

foo = abc2svg.modules.errmsg
foo('error loading ' + m.fn);

I have no such a problem. What is your browser?

bwl21 commented

testing on chrome. I could try on firefox if you wish.

bwl21 commented

I tried with edit-1.xhtml:

  1. in edit.js:20 rename errmsg to xermsg
  2. ninja
  3. rename or delete MIDI-1.js
  4. open edit-1.xhtml

paste to editor:

X:1
I:MIDI program 12
K:C
C

I get an alert

screenshot_1178

The same happens on firefox and Safari:

Safari says

window error: TypeError: Can only call Window.alert on instances of Window
URL: http://localhost:63342/abc2svg/abc2svg-1.js
Line: 20426

Firefox says

window error: TypeError: 'alert' called on an object that does not implement interface Window.
URL: http://localhost:63342/abc2svg/abc2svg-1.js
Line: 20426
bwl21 commented

Now it works. It seems that the native js - function alert cannot be kept in a variable since it refers to the current value of 'this'. Therefore you had to wrap it in a closure.

Nevertheless, from architectural standpoint I still find abc2svg.modules.get_errmsg a bit weird:

  1. it refers to user which is a side effect occasionally working in edit.js. I guess abc2svg.modules.errmsg should be set directly in the calling application.
  2. there are cases where it returns an empty function. Even this should not happen and is a fail from the integrator. But the empty function hides this. I propose that instead of the empty function, it should return a function that cause the whole thing to die. Maybe it never happens, since console.log is always available.

get_errmsg() is a fallback. It is called when the 'errmsg' argument of modules.load() is not furnished.

  1. 'user' may be defined or not when calling modules.load(). If defined, user.errmsg() should be usable.
    Setting abc2svg.modules.errmsg or adding the 'errmsg' argument was an alternative. It is still time to go the other way. But, anyway, get_errmsg() will remain.

  2. a failure in loading a module is not critical: only the associated feature will not work. Crashing the work is not a good solution.
    console.log is not always available. There is no console at least in the js24 and d8 interpreters.

bwl21 commented

Ok so leave it as it is. The errmsg argument for modules.load is fine (you still need to update doc in wiki).
The fallback now works. I guess you keep it as undocumented feature.

bwl21 commented

I saw that you upddated the wiki accordingly. But I still have a question:

The documentation says:

It returns true when all the needed modules are loaded (no new feature). In this case, the
function ondone() is not called.
When false is returned, the caller is alerted via the callback function ondone().

Does it mean

It returns true when all the needed modules were previously loaded (no new feature). In this
case, the function ondone() is not called.

It returns false if not all modules were previously loaded. In this case the modules are
loaded asynchronuously and caller is alerted via the callback function ondone() in case of
success resp. errmsg() in case of failure).

When true is returned, either:

  • no module is needed or
  • some modules were needed, but, either they are already loaded, or they could not be loaded.

When false is returned, some modules are not fully loaded, so, the treatment cannot continue here.
The treatment can continue when the callback function ondone() is called.
The function errmsg() is called when some problem occurs while loading a module. At this time, some other modules could not be fully loaded.

bwl21 commented

thanks for the clarification.