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: https://github.com/SeleniumHQ/selenium/issues/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 https://github.com/SeleniumHQ/selenium/issues/12549 issue. So command itself did not work by the undefined error.

Maybe we should investigate the reason further. cc @jlipps

[KazuCocoa]

Would https://github.com/appium/appium-remote-debugger/pull/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: https://github.com/SeleniumHQ/selenium/pull/12704#issuecomment-2165215273 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 https://github.com/SeleniumHQ/selenium/issues/12549, I know the root cause. @jlipps want to fix my error on https://github.com/SeleniumHQ/selenium/pull/12704, but once this 12549 fixed, another issue https://github.com/SeleniumHQ/selenium/issues/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 https://github.com/appium/appium-remote-debugger/pull/268 diff. It looks like... this is generated code related, so compiler related issue as https://github.com/SeleniumHQ/selenium/issues/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 https://github.com/appium/appium-remote-debugger/pull/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
    • https://github.com/bazelbuild/rules_closure/commit/7434c41542ca9e1b05166d897b90073d1b8b2cf8

• 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.