Missing user prompt checks for most commands

RESOLVED FIXED

Status

defect
P1
normal
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: ato, Unassigned)

Tracking

(Blocks 1 bug)

Version 3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Most commands in Marionette are missing checks for any open user prompts.
Blocks: webdriver
Duplicate of this bug: 1206126
Hm, the spec is not that clear about that but I assume that closing an alert should by synchronous? Right now I do not see that we are actually waiting for the alert being closed before continuing. As result we have lots of wait for conditions in the unit tests. David, should we be more explicit in the spec?

https://w3c.github.io/webdriver/webdriver-spec.html#user-prompts
Flags: needinfo?(dburns)
(In reply to Henrik Skupin (:whimboo) from comment #2)
> Hm, the spec is not that clear about that but I assume that closing an alert
> should by synchronous? Right now I do not see that we are actually waiting
> for the alert being closed before continuing. As result we have lots of wait
> for conditions in the unit tests. David, should we be more explicit in the
> spec?
> 
> https://w3c.github.io/webdriver/webdriver-spec.html#user-prompts

All commands are supposed to be synchronous when they return to the local end from the remote end.
Flags: needinfo?(dburns)
Ok, so that is clearly a point which doesn't apply to our code yet. I will take care of that when working on this bug.
For what it’s worth, I don’t think the spec needs to change.  It can be assumed from the way it is written that user prompts are expected to be closed synchronously.
[mass] Setting priority
Priority: -- → P1
Depends on: 1359004
I think we do this pretty consistently now.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Mostly fixed by the patch on bug 1416284, but I still see missing calls to `_assertAndDismissModal` by certain commands like `executeScript`, and `executeAsyncScript`. Also there might be others too.
Status: RESOLVED → REOPENED
Depends on: 1416284
Resolution: FIXED → ---
Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1439995 to follow up on those commands.
Status: REOPENED → RESOLVED
Closed: 2 years agoLast year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.