Danp2 / au3WebDriver

Web Driver UDF for AutoIt
MIT License
108 stars 23 forks source link

Expand _WD_FrameList with additional parameters #420

Closed Danp2 closed 1 year ago

Danp2 commented 1 year ago

btw. I have also a plan to: Add support for checking all frames in _WD_LoadWait but firstly 362 _WD_FrameList must be merged.

Originally posted by @mlipok in https://github.com/Danp2/au3WebDriver/issues/385#issuecomment-1225742545

mlipok commented 1 year ago

Currently _WD_FrameList() in the internal function __WD_FrameList_Internal() already uses _WD_LoadWait().

https://github.com/Danp2/au3WebDriver/blob/f241f2b34daee457fa39e2c4cf538ca670f95ddc/wd_helper.au3#L853-L868

So can we use this feature and parametrize usage of _WD_LoadWait() within _WD_FrameList() ?

ps. assign this issue to me.

Danp2 commented 1 year ago

So can we use this feature and parametrize usage of _WD_LoadWait() within _WD_FrameList() ?

I don't understand what it is you want to do here. Can you explain further please?

mlipok commented 1 year ago

In line 864 https://github.com/Danp2/au3WebDriver/blob/f241f2b34daee457fa39e2c4cf538ca670f95ddc/wd_helper.au3#L860-L868

w are already using _WD_LoadWait() function thus _WD_FrameList() waiting for each frame.

I think we can use this part of code by adding few parameters to _WD_FrameList() Something like

Func _WD_FrameList($sSession, $bReturnAsArray = True, $iDelay = Default, $iTimeout = Default, $sElement = Default, $iState = Default)

and in this way make it easy to solve this Issue/FeatureRequest Check frames in _WD_LoadWait

What you think about ?

Danp2 commented 1 year ago

I'm still not following your logic. The issue's title is "Check frames in _WD_LoadWait", so I was expecting a proposal where _WD_LoadWait was modified to perform additional steps. This doesn't appear to be the case. Hence, my confusion.

mlipok commented 1 year ago

I was expecting a proposal where _WD_LoadWait was modified to perform additional steps.

ok, lets split your expectation to 2 separate steps

STEP 1. perform additional steps

I'm talking here that this additional steps can be just to use:

Func _WD_FrameList($sSession, $bReturnAsArray = True, $iDelay = Default, $iTimeout = Default, $sElement = Default, $iState = Default)

STEP 2. proposal where _WD_LoadWait was modified

in a future as a one of next step just after the first one

Danp2 commented 1 year ago

Here's what I'm thinking without putting too much effort into it --

mlipok commented 1 year ago
  • It seems like you want to use _WD_FrameList to perform a task that is outside it's intended purpose.

It already uses _WD_LoadWait to get better results, We can add few additionall parameters:

Func _WD_FrameList($sSession, $bReturnAsArray = True, $iDelay = Default, $iTimeout = Default, $sElement = Default, $iState = Default)

To give user a possibility to decide how long he want to wait.

  • It doesn't make sense to me to have _WD_LoadWait call _WD_FrameList, which then turns around and calls _WD_LoadWait

it is requrent calls for each separate frame and sub frames

  • Perhaps we shouldn't be modifying _WD_LoadWait at all, and instead add a separation function to perform this new action.

ok, lets keep hand off from _WD_LoadWait and instead just add few additionall parameters:

Func _WD_FrameList($sSession, $bReturnAsArray = True, $iDelay = Default, $iTimeout = Default, $sElement = Default, $iState = Default)

This will lead to what I said before: To give user a possibility to decide how long he want to wait.

Would it help to gather the loading status of each frame into the array produced by _WD_FrameList?

using $_WD_READYSTATE_*** and this:

$sReadyState = _WD_ExecuteScript($sSession, 'return document.readyState', '', Default, $_WD_JSON_Value)

?

Danp2 commented 1 year ago

ok, lets keep hand off from _WD_LoadWait and instead just add few additional parameters:

Func _WD_FrameList($sSession, $bReturnAsArray = True, $iDelay = Default, $iTimeout = Default, $sElement = Default, $iState = Default)

I'm good with not touching _WD_LoadWait. I think you should evaluate each of the proposed new parameters for _WD_FrameList. For example --

using $_WDREADYSTATE*** and this:$sReadyState = _WD_ExecuteScript($sSession, 'return document.readyState', '', Default, $_WD_JSON_Value)

Yes. But it appears that _WD_LoadWait already set @extended to the index of $aWD_READYSTATE, so you should be able to use this information without having to call _WD_ExecuteScript again.

mlipok commented 1 year ago

I'm good with not touching _WD_LoadWait. I think you should evaluate each of the proposed new parameters for _WD_FrameList. For example --

  • $iDelay - Does this occur only once at the beginning of the function or for each frame?

IMO should occurs only once. Will check it.

  • $iTimeout - Is this per frame or for the entire function call?

IMO per frame. Will check it.

  • $sElement - This one makes no sense IMO

You have right there is no sens for this,

as a side note: I just copy paste parameters from other function to show POC - proof of concept ;)

using $_WDREADYSTATE*** and this:$sReadyState = _WD_ExecuteScript($sSession, 'return document.readyState', '', Default, $_WD_JSON_Value)

Yes. But it appears that _WD_LoadWait already set @Extended to the index of $aWD_READYSTATE, so you should be able to use this information without having to call _WD_ExecuteScript again.

oh. thanks for pointing me to this.

mlipok commented 1 year ago

but before we do next step

in first step please focus on: https://github.com/Danp2/au3WebDriver/pull/434

and then https://github.com/Danp2/au3WebDriver/pull/433