MarketSquare / robotframework-browser

Robot Framework Browser library powered by Playwright.
Apache License 2.0
526 stars 106 forks source link

RFB can not handle two consecutives alerts #3785

Closed wojtekwp closed 1 month ago

wojtekwp commented 1 month ago

Describe the bug RFB not able to handle two consecutives alerts. It fails with below message:

TimeoutError: page.waitForEvent: Timeout 5000ms exceeded while waiting for event “dialog” =========================== logs =========================== waiting for event “dialog” Tip: Use “Set Browser Timeout” for increasing the timeout.

To Reproduce I prep 2 simple files to easily reproduce an issue. Create new folder and copy 2 files content ex. to c:/alert_demo, then run start CMDs

cd c:/alert_demo
robot -d example/1 -t Alert-Debug example.robot
<html lang="pl">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Multiple alerts issue - example</title>
</head>
<body>
    <button onclick="showAlerts()">Click to show alerts</button>

    <script>
        function showAlerts() {
            confirm("First alert accepted?");
            alert("Second alert!");
        }
    </script>
</body>
</html>

Test Cases Alert-Debug Browser Setup

provide correct index.html path below

Go To    C:/alert_demo/index.html
Click    //button
Validate Alert Response    First alert accepted?
Validate Alert Response    Second alert!

Keywords Browser Setup Set Browser Timeout 30 New Browser browser=chromium headless=false args=["--start-maximized"] New Context viewport=${None} New Page url=about:blank

Validate Alert Response [Arguments] ${messageValue} ${message} = Wait For Alert action=accept text=${messageValue} timeout=5



**Expected behavior**
Two alerts should be handled correctly

**Desktop :**
chromium
Win 11 E
Python 3.11.9
NodeJS v20.15.1
Browser library 18.6.3
Robot Framework version: 7.0.1
Playwright 1.45.0

**Additional context**
More details: https://forum.robotframework.org/t/can-not-handle-two-consecutives-alerts/3347
aaltat commented 1 month ago

@allcontributors please add @wojtekwp for bugs.

allcontributors[bot] commented 1 month ago

@aaltat

I've put up a pull request to add @wojtekwp! :tada:

aaltat commented 1 month ago

Playwright does not allow to register two dialog handlers and therefore it crashes. I think that is by design. But I can also see a problem, because if you do only this:

    ${promise} =    Promise To    Wait For Alert    action=accept    timeout=5
    Click    id=alert
    ${text} =    Wait For      ${promise}
    Log    ${text}

You get only text from first alter and not from the second on. The second on is automatically dismissed by Playwright.

The problem can be solved by changing the code to of the page to call multiple times waitForEvent, but problem is that current keyword does not allow one to define different actions or expect different text for the alert. Like your example, you are excepting alerts to have different texts.

To solve problem correctly, we would need new keyword which would allow handling one or more alerts. Also that keyword should allow handling each alert differently, meaning have different values for action, prompt_inputand text (I think same timeout can be used for all alerts.) But what would be good name and syntax for this new keyword?

One option could be to kwargs and suffix each alert handling with number. Example this would handle two different alerts:

    ${promise}    Promise To    Wait For Alerts    action1=accecpt    text1=Text One Here    action2=dismiss    text2=Two is here
    Click    id=alert
    ${text} =            Wait For      ${promise}    # ${text} would be list of two string

But there could better ways to define handling of multiple alerts and I am open to ideas? Would you idea for keyword syntax?

wojtekwp commented 1 month ago

I tried to add below snippet to Browser\keywords\interaction.py and prep next rfb example code with declared list variable & promise but it not always works as expected - don't know why but sometimes still failing to catch and retrieve response from dialog within expected 5s timeout.

If this "Wait For Alerts" keyword fails please try several times - it's not stable - works as expected usually 4/10 tries, sometimes more.

Variables @{ACTIONS} accept dismiss @{TEXTS} First alert accepted? Second alert!

@{TEXTS} First alert acceptedd? Second alerttt!

Test Cases Alert-Debug Browser Setup

provide correct index.html path below

Go To    C:/alert_demo/index.html
Click    //button
Validate Alert Response    First alert accepted?
Validate Alert Response    Second alert!

Alert-Debug2 Browser Setup

provide correct index.html path below

Go To    C:/alert_demo/index.html
${promise} =    Promise To    Wait For Alerts    actions=${ACTIONS}    texts=${TEXTS}    timeout=5  
Click    //button
${texts} =    Wait For    ${promise}
Log    ${texts}

Keywords Browser Setup Set Browser Timeout 30 New Browser browser=chromium headless=false args=["--start-maximized"] New Context viewport=${None} New Page url=about:blank

Validate Alert Response [Arguments] ${messageValue} ${message} = Wait For Alert action=accept text=${messageValue} timeout=5



- execute from cmd with 

> cd c:/alert_demo
> robot -d example/2 -t Alert-Debug2 example.robot
aaltat commented 1 month ago

There is also changes needed in the node side. In any case, your way to declare keyword syntax is valid one, because then automatic argument conversion should work. The downside might be that if there are many alerts, seeing what arguments belongs to which alert may come hard. But I think that is pretty rare case, because handling many alerts is not that common. I think if better ideas don’t arrive, I will go with this approach.