Danp2 / au3WebDriver

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

_WD_GetTable - returns the rows of the nested table #496

Closed mlipok closed 1 year ago

mlipok commented 1 year ago

Bug report

Describe the bug

When a Table contains a nested table, then _WD_GetTable also returns the rows for the nested table.

How to reproduce

try to get table from here: https://www.w3schools.com/howto/tryit.asp?filename=tryhow_html_nested_table

Expected behavior

The result should only include the outer/selected/desired set of table rows, not also the contents of the nested table.

Screenshots

Additional context

none

System under test

not related

Fix proposal

;~  If $sRowsSelector = Default Then $sRowsSelector = "tr"
    If $sRowsSelector = Default Then $sRowsSelector = ":scope > tbody > tr"
mlipok commented 1 year ago

Question: Should we do the same with:

    If $sColsSelector = Default Then $sColsSelector = "td, th"
    If $sColsSelector = Default Then $sColsSelector = ":scope > tbody > tr > td, :scope > tbody > tr > th"

?

mlipok commented 1 year ago

btw. ; https://stackoverflow.com/questions/64842157 should be changed to: ; https://stackoverflow.com/a/64842239/5314940

mlipok commented 1 year ago

@Danp2 I'm waiting for your opinion here.

Danp2 commented 1 year ago

Your proposed fix didn't produced the desired results for me. Here's my test code --

; The contents of this file are read & executed by the UserFile() function in wd_demo.au3
; Changes can be made to this file and quickly tested without having to exit or even close browser
ConsoleWrite("! Code now executing from usertesting.au3" & @CRLF)

ConsoleWrite("- Test 2:" & @CRLF)
_WD_NewTab($sSession, True)
_WD_Navigate($sSession, 'https://www.w3schools.com/howto/tryit.asp?filename=tryhow_html_nested_table')
_WD_LoadWait($sSession)

__SetVAR(0, _WD_FindElement($sSession, $_WD_LOCATOR_ByXPath, "//iframe[@id='iframeResult']"))
_WD_FrameEnter($sSession, $_VAR[0])

__SetVAR(1, _WD_GetTable($sSession, "//table"))
_ArrayDisplay($_VAR[1])

__SetVAR(2, _WD_GetTable($sSession, "//table"), ":scope > tbody > tr")
_ArrayDisplay($_VAR[2])

FWIW, I'm not sure that we need to change anything here since the existing functionality works in most cases and the user can override the selector as needed.

Danp2 commented 1 year ago

btw. ; https://stackoverflow.com/questions/64842157 should be changed to: ; https://stackoverflow.com/a/64842239/5314940

One takes you the original question while the other takes you to the solution. Either way, you end up at the same web page that shows where the idea / code came from.

mlipok commented 1 year ago

Your proposed fix didn't produced the desired results for me. Here's my test code --

becuase you have issue in the code

bad line:

__SetVAR(2, _WD_GetTable($sSession, "//table"), ":scope > tbody > tr")

fixed line:

__SetVAR(2, _WD_GetTable($sSession, "//table", ":scope > tbody > tr"))

entire example

; The contents of this file are read & executed by the UserFile() function in wd_demo.au3
; Changes can be made to this file and quickly tested without having to exit or even close browser
ConsoleWrite("! Code now executing from usertesting.au3" & @CRLF)

ConsoleWrite("- Test 2:" & @CRLF)
_WD_NewTab($sSession, True)
_WD_Navigate($sSession, 'https://www.w3schools.com/howto/tryit.asp?filename=tryhow_html_nested_table')
_WD_LoadWait($sSession)

__SetVAR(0, _WD_FindElement($sSession, $_WD_LOCATOR_ByXPath, "//iframe[@id='iframeResult']"))
_WD_FrameEnter($sSession, $_VAR[0])

__SetVAR(1, _WD_GetTable($sSession, "//table"))
_ArrayDisplay($_VAR[1], 'Test 1')

__SetVAR(2, _WD_GetTable($sSession, "//table", ":scope > tbody > tr"))
_ArrayDisplay($_VAR[2], 'Test 2')
mlipok commented 1 year ago

I'm not sure that we need to change anything here since the existing functionality works in most cases and the user can override the selector as needed.

so at least remark in function header description about nested <table> element

Danp2 commented 1 year ago

so at least remark in function header description about nested <table> element

That works for me.

":scope > tbody > tr > td, :scope > tbody > tr > th"

What are you expecting the results to be with this? I got an empty array with this code --

__SetVAR(1, _WD_GetTable($sSession, "//table"))
_ArrayDisplay($_VAR[1], "Standard")

__SetVAR(2, _WD_GetTable($sSession, "//table", ":scope > tbody > tr"))
_ArrayDisplay($_VAR[2], "Modified #1")

__SetVAR(3, _WD_GetTable($sSession, "//table", ":scope > tbody > tr", ":scope > tbody > tr > td, :scope > tbody > tr > th"))
_ArrayDisplay($_VAR[3], "Modified #2")
mlipok commented 1 year ago

so at least remark in function header description about nested <table> element

That works for me.

I don't know what you meant here

mlipok commented 1 year ago

":scope > tbody > tr > td, :scope > tbody > tr > th"

What are you expecting the results to be with this? I got an empty array with this code --

I was just wondering, but now I understand few minutes ago how this following code works

        Local $sScript = "return [...arguments[0].querySelectorAll(arguments[1])]" & _
                ".map(row => [...row.querySelectorAll(arguments[2])]" & _
                ".map(cell => cell.textContent));"

thanks to this article and video https://www.freecodecamp.org/news/javascript-map-how-to-use-the-js-map-function-array-method/

Danp2 commented 1 year ago

I don't know what you meant here

I was agreeing with your assessment that adding a remark to the function header would be a good alternative to actually changing the function's defaults.

Danp2 commented 1 year ago

; Description ...: Return all elements of a table.

I suggest that we also review / modify this description. Technically, it is returning the text content of the elements, not the elements themselves. What do you think about this?

; Description ...: Retrieve text from all matching elements of a table.

mlipok commented 1 year ago

I suggest that we also review / modify this description. Technically, it is returning the text content of the elements, not the elements themselves. What do you think about this?

; Description ...: Retrieve text from all matching elements of a table.

Description changed