Workarena/Base: Merge init scripts before evaluation.#83
Workarena/Base: Merge init scripts before evaluation.#83conte91 wants to merge 3 commits intoServiceNow:mainfrom
Conversation
| page.evaluate(init_script_js) | ||
| for f in page.frames: | ||
| f.evaluate(init_script_js) |
There was a problem hiding this comment.
I think the evaluate lines should be removed
|
Self-note: this could be an issue in case names defined across scripts shadow each other? Something like: I think we should be fine though, because even if two scripts' private functions will share the same name, the last one defined will win. |
remove evaluate
|
@conte91 Found a regression introduced by this PR — chart tasks ( Fix is to add these 3 lines to WorkArena/src/browsergym/workarena/tasks/utils/js_utils.js Lines 37 to 39 in ff7aebd // Explicitly expose on window so it is accessible via frame.evaluate_handle()
// when all init scripts are merged into a single string (see base.py).
window.findElementInShadowDOM = findElementInShadowDOM;Verified with Please add these 3 lines and i'll merge as soon as this is included! |
Fixes #57 and duplicates without the need for downgrading the playwright version.
Playwright's init scripts are not executed sequentially, as per PW's docs:
However, some benchmarks depend on all installed init scripts to be executed in order, which is why the page hangs if they don't (in WorkArena's case, this was because of the function being waited on being undefined - since the script defining it hadn't been evaluated yet).
It shouldn't cause any problem to just concatenate all scripts together, so that we can guarantee FIFO execution order.