Closed Bug 1279211 Opened 8 years ago Closed 6 years ago

Missing user prompt checks for most commands

Categories

(Remote Protocol :: Marionette, defect, P1)

Version 3
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ato, Unassigned)

References

(Blocks 1 open bug)

Details

Most commands in Marionette are missing checks for any open user prompts.
Blocks: webdriver
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: 6 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: 6 years ago6 years ago
Resolution: --- → FIXED
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.