regression issue from selenium, seems forget to re-build atoms

[Jabbar2010]

This issue is just a bug I found, so no demo can be provided.

As you can see, the latest file history of "atoms/execute_script.js" below is updated 10 months ago, the first argument of the outer function is window, but the previous one is { navigator: typeof window != 'undefined' ? window.navigator : null, document: typeof window != 'undefined' ? window.document : null }

I found selenium has updated this wrapper function https://github.com/SeleniumHQ/selenium/pull/12704/files, but not in appium-remote-debugger, did you members not re-build "npm run build:atoms"?

The original issue is here: SeleniumHQ/selenium#12659, so I also had the same issue when I used appium-xcuitest-driver on iOS Safari.

[KazuCocoa]

Current our atoms have https://github.com/appium/appium-remote-debugger/blob/master/atoms-notes.md since the vanilla atom build module had SeleniumHQ/selenium#12549 issue. So command itself did not work by the undefined error.

Maybe we should investigate the reason further. cc @jlipps

[KazuCocoa]

Would #384 work for you? I just build atoms from selenium trunk.

[Jabbar2010]

Thanks @KazuCocoa , I checked the build files, and I think it's ok, but now I have another issue: SeleniumHQ/selenium#12704 (comment)
Besides document and navigator, in some files, it needs other apis such as window.JSON,window.Range, so I don't know how to fix it.

[Jabbar2010]

Hi, @KazuCocoa , according to SeleniumHQ/selenium#12549, I know the root cause. @jlipps want to fix my error on SeleniumHQ/selenium#12704, but once this 12549 fixed, another issue SeleniumHQ/selenium#12659 happened.

So is there another way to change "_ " which conflicts with lodash inner the function to other name just like "___"?(I didn't find where the code in the screenshot is from)

[Jabbar2010]

Hi, any ideas for this?

[ahalbrock]

The real issue seems to be why the driving function from the atom is being assigned to this._ in the first place before returning the result. Why do:
this._ = mainFunc; return this._.apply(null, arguments);

When the following should achieve the same result without clobbering a global:
return mainFunc.apply(null, arguments);

The previous use of { navigator: window.navigator, document: window.document } as "this" instead of "window" masked this issue since the new object didn't have an underscore to clobber.

The atoms seem to follow 2 general formats for the assignment of the driving function to this.. The for loop that was shown earlier in the comments, and another where there is a function right before the "return this..apply(...)" that takes a string and a function and basically assigns the function to this[theString]. For the first case I've just gotten rid of the for loop and changed the return this._.apply to use whatever function was going to be assigned to _ in the for loop, e.g. return V.apply(null, arguments), and for the second I removed the function call and gave the anonymous function a name of RET_FUNC or whatever you like and then updated the return to "return RET_FUNC.apply(null, arguments)"

This fixes the issue for me but needs to be done in all the atoms as they ALL seem to clobber this._ in this way. I am not familiar with the code that actually generates these atoms to suggest a fix pre-generation however.

[Jabbar2010]

@KazuCocoa @jlipps we need your help with this, it blocks our testing for the website used lodash( _ ).
Two weeks have gone.

[ahalbrock]

If you hand modify the atoms as I mentioned above, they should work as intended and not clobber your lodash in the process.

For instance, this is the last line of the execute_script.js atom in the atoms directory:

for(var Y;W.length&&(Y=W.shift());)W.length||void 0===V?X[Y]&&X[Y]!==Object.prototype[Y]?X=X[Y]:X=X[Y]={}:X[Y]=V;; return this._.apply(null,arguments);}).apply(window, arguments);}

If you modify that line to be just the following:
return V.apply(null,arguments);}).apply(window, arguments);}

(modify the return to use "V" instead of this._ and remove the for loop, the atom should function as intended.)

There is this format where the for loop may assign a different function at the end, and there is another where instead you get something like the following (this function may get a different generated name, but looks the part):

aa("", function() {...});; return this..apply(null, arguments);}).apply(window, arguments);}

For these I pull out the anonymous function and give it a name, then use that for the return:

function MAIN_FUNC () {...}; return MAIN_FUNC.apply(null, arguments);}).apply(window, arguments);}

If you modify all atoms in this way (they all have one of those patterns to assign the main function to this._) then you should avoid clobbering lodash.

[KazuCocoa]

Read #268 diff. It looks like... this is generated code related, so compiler related issue as SeleniumHQ/selenium#12549 guess. I don't have knowledge, but we may need to check the closure compiler or Bazel's compilation stuff in selenium (e.g. compiler version etc)

var rc=ba.JSON.stringify to var rc=typeof ba === 'undefined' ? ba.JSON.stringify : JSON.stringify in find_element_fragments.js based on #384 succeeded in calling driver.find_element.

[KazuCocoa]

• https://github.com/bazelbuild/rules_closure

• selenium v3.150.0 <- this could be wrong. v3.14 and lower was duck, so this might be wrong.

  • https://github.com/SeleniumHQ/selenium/blob/selenium-3.150.0/WORKSPACE#L11-L20
    • bazelbuild/rules_closure@7434c41

• selenium v3.14

  • https://github.com/SeleniumHQ/selenium/tree/selenium-3.141.59/third_party/closure

• selenium v4

  • https://github.com/SeleniumHQ/selenium/blob/trunk/WORKSPACE#L7-L20

I wondered if it would be worth trying out the newer/older closure JS compiler for Bazel build. I was able to build with rules_closure-0.12.0, but they did not have any diff. I assume building them with older one may have different output.