_WD_ExecuteScript: don't reformat JS code

Danp2 commented 1 year ago

Pull request

Proposed changes

Fixes #487

Checklist

Put an x in the boxes that apply. If you're unsure about any of them, don't hesitate to ask. We are here to help!
This is simply a reminder of what we are going to look for before merging your code.

[x] I have read and noticed the CODE OF CONDUCT document
[x] I have read and noticed the CONTRIBUTING document
[ ] I have added necessary documentation or screenshots (if appropriate)

Types of changes

Please check x the type of change your PR introduces:

[x] Bugfix (change which fixes an issue)
[ ] Feature (change which adds functionality)
[ ] Code style update (formatting, renaming)
[ ] Refactoring (functional, structural)
[ ] Documentation content changes
[ ] Other (please describe)

What is the current behavior?

Stripping of tabs and spaces from JS can result in code that no longer functions properly.

What is the new behavior?

Code is processed as-is. Any reformatting that is required should be performed prior to calling _WD_ExecuteScript

Additional context

System under test

mlipok commented 1 year ago

still need to check, but currently focusing on frames issue.

mlipok commented 1 year ago

still need to check, but currently focusing on frames issue.

here is my latest opinion

https://github.com/Danp2/au3WebDriver/issues/487#issuecomment-1659380663

mlipok commented 1 year ago

Please remove all $JSON_MLREFORMAT usage

mlipok commented 1 year ago

Also in wd_helper.au3 the use of StringReplace(...... , @TAB, '') should be considered a candidate for removal.

mlipok commented 1 year ago

Also in wd_helper.au3 the use of StringReplace(...... , @TAB, '') should be considered a candidate for removal.

because:

$sData = Json_StringEncode($sData, $iOption) ; Escape JSON Strings

from here: https://github.com/Danp2/au3WebDriver/blob/c39d1f56fee86a5ebe5c9a9f9082ccdea120a5f2/wd_core.au3#L1744-L1748

It's enough

mlipok commented 1 year ago

The other question is why we always return $_WD_ERROR_Success here: https://github.com/Danp2/au3WebDriver/blob/c39d1f56fee86a5ebe5c9a9f9082ccdea120a5f2/wd_core.au3#L1747

mlipok commented 1 year ago

Also in wd_helper.au3 the use of StringReplace(...... , @TAB, '') should be considered a candidate for removal.

but maybe not? I think they are there to CleanUp the mess in sphagetthi called logs.

Danp2 commented 1 year ago

Also in wd_helper.au3 the use of StringReplace(...... , @TAB, '') should be considered a candidate for removal.

To me, these are two separate issues:

1) JS code submitted by the user 2) JS code supplied internally by the UDF

This PR is intended to only deal with the 1st instance.

but maybe not? I think they are there to CleanUp the mess in sphagetthi called logs.

Right... IIRC, you wanted the inline code to be more legible so this is the way it was done.

Danp2 commented 1 year ago

The other question is why we always return $_WD_ERROR_Success here:

IDK. We are dealing with an internal function, so it may not matter. This appears to be a carry over from before we switched to using Json_StringEncode. I suppose we could check the error code from Json_StringEncode, but then return a different error code if the function returned an error.

FWIW, I also noticed that the function header is missing a Return values entry.

mlipok commented 1 year ago

The other question is why we always return $_WD_ERROR_Success here:

https://github.com/Danp2/au3WebDriver/blob/c39d1f56fee86a5ebe5c9a9f9082ccdea120a5f2/wd_core.au3#L1747

This is the last question here

mlipok commented 1 year ago

. I suppose we could check the error code from Json_StringEncode, but then return a different error code if the function returned an error.

Please propose solution.I will check when I woke up.

mlipok commented 1 year ago

Looks fine for me

Danp2 / au3WebDriver