Closed
Bug 1279211
Opened 8 years ago
Closed 6 years ago
Missing user prompt checks for most commands
Categories
(Remote Protocol :: Marionette, defect, P1)
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.
Comment 2•7 years ago
|
||
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)
Comment 3•7 years ago
|
||
(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)
Comment 4•7 years ago
|
||
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.
Reporter | ||
Comment 5•7 years ago
|
||
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.
Reporter | ||
Comment 7•6 years ago
|
||
I think we do this pretty consistently now.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment 8•6 years ago
|
||
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.
Reporter | ||
Comment 9•6 years ago
|
||
Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1439995 to follow up on those commands.
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•