Testing: Safari & IE8

[krusynth]

For Safari and IE8 only, my unit tests are throwing an error complaining about the "after each" hook, which is showing up in legacy_ranges_spec.js on line 575 - this maps to backbone-events-standalone.js line 127, the _.keys call is undefined. I'm assuming this is because there's an underscore dependency that's not being resolved - but it's strange that only these two browsers are complaining. I'm digging in a bit more.

[aron]

@krues8dr the standalone lib doesn't use underscore, and it looks like it doesn't support IE8 due to Object.keys. See: https://github.com/n1k0/backbone-events-standalone/blob/master/backbone-events-standalone.js#L34

Although it looks like other ES5 methods are shimmed, so if it's just Object.keys that was an oversight they might accept a patch.

[aron]

Looks like there was even a patch offered by the author of a similar library eventify. n1k0/backbone-events-standalone#6. Perhaps we could use that.

[krusynth]

That did the trick, this is just pending acceptance of that PR.

[krusynth]

Ok, further issues that are probably somewhat related. Now there's a failure at Adder plugin "before each" hook : Object expected, which refers to this block.

[aron]

Anything more specific? A line number or a stack trace? Otherwise best thing to do is remove all the lines and add them back until it throws…

[krusynth]

It's throwing neither - and stepping through in the debugger has so many issues that it's taking forever to get to something meaningful. Doing the latter now.

[krusynth]

Appears to be core = new Annotator.Delegator(el: h.fix())

[krusynth]

Ah, Annotator.Delegator is undefined.

[krusynth]

Ah, it's undefined in every browser now. I clearly have broken something else.

Edit: fixing that in Chrome hasn't fixed it in IE still. Something else is still going on.

[tilgovi]

@krues8dr thanks for the reports. This is great.

[krusynth]

So, for IE8, Annotator.Delegator remains undefined. If I import it manually via Delegator = require('../../../src/delegator') It works far enough to get to the plugin.configure call, where it throws Object doesn't support this action. Looking at the plugin in question, Adder, we find a call to Object.defineProperty, which is only partially implemented in IE8 - it only works on DOM objects for IE8, and kills the whole page execution on anything else. That method is showing a deprecation warning in the first place, so I assume it's going away altogether.

I'm not seeing any documentation for using the Adder plugin, so I'm not sure what the ideal way to refactor this would be. @tilgovi : it looks like you made that addition - what's the best way forward with these tests using this new method?

[aron]

I would just check for Object.defineProperty before defining the deprecation warning and skip it if not available. I think we're fine with IE8 not logging warnings.

[krusynth]

The issue is that Object.defineProperty is defined - it just doesn't behave as expected.

[aron]

Oh of course I'm sorry. We could add a helper that does the assignment and wraps it in a try/catch. But that's pretty nasty too.

[krusynth]

Well, since this is already deprecated as a method, surely there's a better implementation we could use for unit testing purposes at least? And then just mark it as a known issue for anyone moving forward.

[tilgovi]

Hmmm. I think I support simply removing this stuff from the adder, editor, and viewer. I don't think backward compatibility for these properties is of utmost importance.

[krusynth]

Great, but for the purposes of fixing the tests, I still need a bit more guidance. E.g., for the editor tests, here's what's currently calling configure:

  plugin = new Editor()
  plugin.configure({core: core})
  plugin.pluginInit()

Which I assumed was just setting core.editor to plugin, so I used this:

  plugin = new Editor()
  core.editor = plugin
  plugin.pluginInit()

But that fails with TypeError: Cannot read property '_listenerId' of undefined from here, so something is missing from the events that core should have?

[krusynth]

Oh wait - @core, right. So there's some weird circular dependency being setup there for some reason. This worked:

  plugin = new Editor()
  plugin.core = core
  core.editor = plugin
  plugin.pluginInit()

A few new issues after getting that running in IE8, tracking those now.

[tilgovi]

Yep. The editor property on the core is deprecated, but some plugins do
need a reference to the core.
On Aug 14, 2014 1:33 PM, "Bill Hunt" notifications@github.com wrote:

Oh wait - @core, right. So there's some weird circular dependency being
setup there for some reason. This worked:

plugin = new Editor()
plugin.core = core
core.editor = plugin
plugin.pluginInit()

A few new issues after getting that running in IE8, tracking those now.

—
Reply to this email directly or view it on GitHub
#408 (comment)
.

[krusynth]

Circling back to the original thread that started this all, n1k0/backbone-events-standalone#10 has been accepted so backbone-events-standalone is now 👍 for IE8.

[nickstenning]

Is there anything more to do here?

[krusynth]

No, I think all of the original issues here have been resolved from #412 mostly.