search

refactoror / SelBlocks

SelBlocks extension for Selenium IDE
10 stars 7 forks source link

Implementation of interceptBefore() and interceptAfter() #3

Closed peter-lyons-kehl closed 9 years ago

peter-lyons-kehl commented 10 years ago

Hi Chris,

in function-intercepting.js of SelBlocks 2.0.1, both $$.fn.interceptBefore = function(targetObj, targetFnName, _fn) {...} and $$.fn.interceptAfter = function(targetObj, targetFnName, _fn) {...} have the first line

var existing_fn = targetObj[targetFnName] = _fn;

I believe you've intended to

  1. save original targetObj[targetFnName] to existing_fn
  2. set targetObj[targetFnName] to a new function, which calls the previous implementation (existing_fn) and _fn, in one sequence or the other

For that this needs the first lineof both interceptor functions to be var existing_fn = targetObj[targetFnName];

Current implementation sets both existing_fn and targetObj[targetFnName] to _fn, thus throwing away the reference to the original implementation. I'm not sure how it can currently work and that this didn't cause problems so far. (It caused problems for me when merging SelBlocks 2.0.1 to SelBlocksGlobal).

refactoror commented 10 years ago

That's really strange, Peter. I agree with your analysis, and yet the entire SelBlocks test suite works as is. However, the entire test suite continues to work with your suggested fix.

Now for one thing, interceptBefore() is not currently being used anywhere, and interceptAfter() is used only to extend the behavior of Selenium.reset(). My conclusion, based on much experimentation, is that reset() completely recreates the Selenium object, and therefore the restore aspect is redundant in that case. But surely this code would be a problem in other situations.

So, this is not a functional bug for SelBlocks, but both functions will be corrected in the next dot release, since it is an internal bug.

refactoror commented 9 years ago

This has been fixed in 2.0.2