Skip to content

Commit a48cc97

Browse files
goosewobblerclaude
andauthored
fix(actions): tick duration = max(pause) across sources, not sum (#497)
* fix(actions): tick duration = max(pause) across sources, not sum (#496) W3C WebDriver Actions §17.4.3 defines a tick's duration as the *maximum* pause/move/scroll duration across its sources, not the sum. Both embedded drivers slept each source's duration sequentially within a tick, so a tick with pause(100) in one source and pause(200) in another took 300ms instead of the required 200ms. Both handlers now compute the tick duration up front via a pure `ActionSequence::duration_at(tick)` helper (mirroring `action_count`), dispatch the tick's actions without per-source sleeps, and sleep the single max once at the end. Adds a `tick_duration_is_max_across_sources_not_sum` unit test to each crate. Closes #496. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(dioxus): inline tick-end sleep to match the Tauri handler Greptile P2: tick_duration_ms is already a resolved u64, so wrapping it in Some() for sleep_for just to unwrap it again was roundabout. Inline the `if > 0 { tokio::time::sleep(...) }` form (identical to the Tauri handler, keeping the two converged) and drop the now-unused sleep_for helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 66ed853 commit a48cc97

2 files changed

Lines changed: 121 additions & 61 deletions

File tree

  • packages
    • dioxus-embedded-driver/src/server/handlers
    • tauri-plugin-webdriver/src/server/handlers

packages/dioxus-embedded-driver/src/server/handlers/actions.rs

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,30 @@ impl ActionSequence {
6868
ActionSequence::None { actions, .. } => actions.len(),
6969
}
7070
}
71+
72+
/// The duration (ms) this source's action at `tick` contributes to the tick. Pause, pointer-move
73+
/// and wheel-scroll actions carry a duration; everything else is instantaneous (`None`). The tick's
74+
/// duration is the max of these across sources (W3C §17.4.3), not their sum.
75+
fn duration_at(&self, tick: usize) -> Option<u64> {
76+
match self {
77+
ActionSequence::Key { actions, .. } => match actions.get(tick) {
78+
Some(KeyAction::Pause { duration }) => *duration,
79+
_ => None,
80+
},
81+
ActionSequence::Pointer { actions, .. } => match actions.get(tick) {
82+
Some(PointerAction::PointerMove { duration, .. }) | Some(PointerAction::Pause { duration }) => *duration,
83+
_ => None,
84+
},
85+
ActionSequence::Wheel { actions, .. } => match actions.get(tick) {
86+
Some(WheelAction::Scroll { duration, .. }) | Some(WheelAction::Pause { duration }) => *duration,
87+
_ => None,
88+
},
89+
ActionSequence::None { actions, .. } => match actions.get(tick) {
90+
Some(PauseAction::Pause { duration }) => *duration,
91+
_ => None,
92+
},
93+
}
94+
}
7195
}
7296

7397
#[derive(Debug, Deserialize)]
@@ -405,6 +429,9 @@ pub async fn perform(
405429
// click lands). A source with fewer actions contributes nothing on later ticks.
406430
let tick_count = request.actions.iter().map(ActionSequence::action_count).max().unwrap_or(0);
407431
for tick in 0..tick_count {
432+
// W3C §17.4.3: a tick lasts the *maximum* pause/move/scroll duration across its sources (not the
433+
// sum), slept once after the tick's actions are dispatched.
434+
let tick_duration_ms = request.actions.iter().filter_map(|seq| seq.duration_at(tick)).max().unwrap_or(0);
408435
for action_seq in &request.actions {
409436
match action_seq {
410437
ActionSequence::Key { _id: _, actions } => {
@@ -431,7 +458,7 @@ pub async fn perform(
431458
}
432459
eval(key_event_js("keyup", value, modifiers), timeout_ms).await?;
433460
}
434-
KeyAction::Pause { duration } => sleep_for(*duration).await,
461+
KeyAction::Pause { .. } => {}
435462
}
436463
}
437464
}
@@ -485,22 +512,21 @@ pub async fn perform(
485512
}
486513
}
487514
}
488-
PointerAction::PointerMove { x, y, duration, origin } => {
515+
PointerAction::PointerMove { x, y, origin, .. } => {
489516
let (target_x, target_y) =
490517
resolve_origin(&state, &session_id, origin, *x, *y, &pointer_state, timeout_ms).await?;
491518
pointer_state.x = target_x;
492519
pointer_state.y = target_y;
493520
// A move between clicks breaks a double-click chain. element.doubleClick() never moves
494521
// between its two press/release pairs, so this can't suppress a legitimate dblclick.
495522
last_click_pos = None;
496-
sleep_for(*duration).await;
497523
eval(
498524
pointer_event_js("mousemove", pointer_state.x, pointer_state.y, 0, held_mask, modifiers),
499525
timeout_ms,
500526
)
501527
.await?;
502528
}
503-
PointerAction::Pause { duration } => sleep_for(*duration).await,
529+
PointerAction::Pause { .. } => {}
504530
PointerAction::PointerCancel => {
505531
// A pointer cancel aborts the gesture: release this source's buttons without a click
506532
// (there's no MouseEvent equivalent), so drop its tracked state and clear the held mask.
@@ -523,23 +549,26 @@ pub async fn perform(
523549
ActionSequence::Wheel { _id: _, actions } => {
524550
if let Some(action) = actions.get(tick) {
525551
match action {
526-
WheelAction::Scroll { x, y, delta_x, delta_y, duration } => {
527-
sleep_for(*duration).await;
552+
WheelAction::Scroll { x, y, delta_x, delta_y, .. } => {
528553
eval(wheel_event_js(*x, *y, *delta_x, *delta_y), timeout_ms).await?;
529554
}
530-
WheelAction::Pause { duration } => sleep_for(*duration).await,
555+
WheelAction::Pause { .. } => {}
531556
}
532557
}
533558
}
534559
ActionSequence::None { _id: _, actions } => {
535560
if let Some(action) = actions.get(tick) {
536561
match action {
537-
PauseAction::Pause { duration } => sleep_for(*duration).await,
562+
PauseAction::Pause { .. } => {}
538563
}
539564
}
540565
}
541566
}
542567
}
568+
// The tick's actions are dispatched; wait its longest source duration once.
569+
if tick_duration_ms > 0 {
570+
tokio::time::sleep(Duration::from_millis(tick_duration_ms)).await;
571+
}
543572
}
544573

545574
// Persist the final pointer position for later `origin: "pointer"` resolution.
@@ -592,14 +621,6 @@ async fn resolve_origin(
592621
}
593622
}
594623

595-
async fn sleep_for(duration: Option<u64>) {
596-
if let Some(ms) = duration {
597-
if ms > 0 {
598-
tokio::time::sleep(Duration::from_millis(ms)).await;
599-
}
600-
}
601-
}
602-
603624
/// DELETE `/session/{session_id}/actions` — release actions.
604625
pub async fn release(
605626
State(state): State<Arc<AppState>>,
@@ -892,4 +913,23 @@ mod tests {
892913
let ticks = req.actions.iter().map(ActionSequence::action_count).max().unwrap_or(0);
893914
assert_eq!(ticks, 3);
894915
}
916+
917+
#[test]
918+
fn tick_duration_is_max_across_sources_not_sum() {
919+
// W3C §17.4.3: a tick lasts the max pause/move/scroll duration across sources, not their sum.
920+
let req = parse(
921+
r#"{"actions":[
922+
{"type":"none","id":"n","actions":[{"type":"pause","duration":100},{"type":"pause","duration":50}]},
923+
{"type":"pointer","id":"p","actions":[
924+
{"type":"pointerMove","x":0,"y":0,"duration":200},{"type":"pause","duration":30}
925+
]},
926+
{"type":"wheel","id":"w","actions":[{"type":"scroll","x":0,"y":0,"deltaX":0,"deltaY":0,"duration":80}]}
927+
]}"#,
928+
);
929+
let tick_ms = |tick: usize| req.actions.iter().filter_map(|s| s.duration_at(tick)).max().unwrap_or(0);
930+
// tick 0: max(100, 200, 80) = 200 — not the sum (380).
931+
assert_eq!(tick_ms(0), 200);
932+
// tick 1: max(50, 30) = 50 — the wheel source is exhausted and contributes nothing.
933+
assert_eq!(tick_ms(1), 50);
934+
}
895935
}

packages/tauri-plugin-webdriver/src/server/handlers/actions.rs

Lines changed: 65 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,33 @@ impl ActionSequence {
5353
ActionSequence::None { actions, .. } => actions.len(),
5454
}
5555
}
56+
57+
/// The duration (ms) this source's action at `tick` contributes to the tick. Pause, pointer-move
58+
/// and wheel-scroll actions carry a duration; everything else is instantaneous (`None`). The tick's
59+
/// duration is the max of these across sources (W3C §17.4.3), not their sum.
60+
fn duration_at(&self, tick: usize) -> Option<u64> {
61+
match self {
62+
ActionSequence::Key { actions, .. } => match actions.get(tick) {
63+
Some(KeyAction::Pause { duration }) => *duration,
64+
_ => None,
65+
},
66+
ActionSequence::Pointer { actions, .. } => match actions.get(tick) {
67+
Some(PointerAction::PointerMove { duration, .. })
68+
| Some(PointerAction::Pause { duration }) => *duration,
69+
_ => None,
70+
},
71+
ActionSequence::Wheel { actions, .. } => match actions.get(tick) {
72+
Some(WheelAction::Scroll { duration, .. }) | Some(WheelAction::Pause { duration }) => {
73+
*duration
74+
}
75+
_ => None,
76+
},
77+
ActionSequence::None { actions, .. } => match actions.get(tick) {
78+
Some(PauseAction::Pause { duration }) => *duration,
79+
_ => None,
80+
},
81+
}
82+
}
5683
}
5784

5885
#[derive(Debug, Deserialize)]
@@ -173,9 +200,15 @@ pub async fn perform<R: Runtime + 'static>(
173200
.map(ActionSequence::action_count)
174201
.max()
175202
.unwrap_or(0);
176-
// Each source's pause within a tick currently sleeps sequentially (the tick takes their sum);
177-
// W3C §17.4.3 wants the tick to take the max pause across sources. Tracked in #496.
178203
for tick in 0..tick_count {
204+
// W3C §17.4.3: a tick lasts the *maximum* pause/move/scroll duration across its sources (not
205+
// the sum), slept once after the tick's actions are dispatched.
206+
let tick_duration_ms = request
207+
.actions
208+
.iter()
209+
.filter_map(|seq| seq.duration_at(tick))
210+
.max()
211+
.unwrap_or(0);
179212
for action_seq in &request.actions {
180213
match action_seq {
181214
ActionSequence::Key { _id: _, actions } => {
@@ -203,11 +236,7 @@ pub async fn perform<R: Runtime + 'static>(
203236
session.action_state.pressed_keys.remove(value);
204237
}
205238
}
206-
KeyAction::Pause { duration } => {
207-
if let Some(ms) = duration {
208-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
209-
}
210-
}
239+
KeyAction::Pause { .. } => {}
211240
}
212241
}
213242
}
@@ -275,12 +304,7 @@ pub async fn perform<R: Runtime + 'static>(
275304
}
276305
}
277306
}
278-
PointerAction::PointerMove {
279-
x,
280-
y,
281-
duration,
282-
origin,
283-
} => {
307+
PointerAction::PointerMove { x, y, origin, .. } => {
284308
let (target_x, target_y) = match origin {
285309
// No origin (the default) or "viewport": x/y are absolute viewport coords.
286310
None => (*x, *y),
@@ -317,11 +341,6 @@ pub async fn perform<R: Runtime + 'static>(
317341
};
318342
pointer_state.x = target_x;
319343
pointer_state.y = target_y;
320-
if let Some(ms) = duration {
321-
if *ms > 0 {
322-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
323-
}
324-
}
325344
executor
326345
.dispatch_pointer_event(
327346
PointerEventType::Move,
@@ -331,54 +350,35 @@ pub async fn perform<R: Runtime + 'static>(
331350
)
332351
.await?;
333352
}
334-
PointerAction::Pause { duration } => {
335-
if let Some(ms) = duration {
336-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
337-
}
338-
}
353+
PointerAction::Pause { .. } => {}
339354
}
340355
}
341356
}
342357
ActionSequence::Wheel { _id: _, actions } => {
343358
if let Some(action) = actions.get(tick) {
344359
match action {
345-
WheelAction::Scroll {
346-
x,
347-
y,
348-
delta_x,
349-
delta_y,
350-
duration,
351-
} => {
352-
if let Some(ms) = duration {
353-
if *ms > 0 {
354-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
355-
}
356-
}
360+
WheelAction::Scroll { x, y, delta_x, delta_y, .. } => {
357361
executor
358362
.dispatch_scroll_event(*x, *y, *delta_x, *delta_y)
359363
.await?;
360364
}
361-
WheelAction::Pause { duration } => {
362-
if let Some(ms) = duration {
363-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
364-
}
365-
}
365+
WheelAction::Pause { .. } => {}
366366
}
367367
}
368368
}
369369
ActionSequence::None { _id: _, actions } => {
370370
if let Some(action) = actions.get(tick) {
371371
match action {
372-
PauseAction::Pause { duration } => {
373-
if let Some(ms) = duration {
374-
tokio::time::sleep(std::time::Duration::from_millis(*ms)).await;
375-
}
376-
}
372+
PauseAction::Pause { .. } => {}
377373
}
378374
}
379375
}
380376
}
381377
}
378+
// The tick's actions are dispatched; wait its longest source duration once.
379+
if tick_duration_ms > 0 {
380+
tokio::time::sleep(std::time::Duration::from_millis(tick_duration_ms)).await;
381+
}
382382
}
383383

384384
// Persist the final pointer position so a later performActions call with
@@ -488,4 +488,24 @@ mod tests {
488488
.unwrap_or(0);
489489
assert_eq!(ticks, 0);
490490
}
491+
492+
#[test]
493+
fn tick_duration_is_max_across_sources_not_sum() {
494+
// W3C §17.4.3: a tick lasts the max pause/move/scroll duration across sources, not their sum.
495+
let req = parse(
496+
r#"{"actions":[
497+
{"type":"none","id":"n","actions":[{"type":"pause","duration":100},{"type":"pause","duration":50}]},
498+
{"type":"pointer","id":"p","actions":[
499+
{"type":"pointerMove","x":0,"y":0,"duration":200},{"type":"pause","duration":30}
500+
]},
501+
{"type":"wheel","id":"w","actions":[{"type":"scroll","x":0,"y":0,"deltaX":0,"deltaY":0,"duration":80}]}
502+
]}"#,
503+
);
504+
let tick_ms =
505+
|tick: usize| req.actions.iter().filter_map(|s| s.duration_at(tick)).max().unwrap_or(0);
506+
// tick 0: max(100, 200, 80) = 200 — not the sum (380).
507+
assert_eq!(tick_ms(0), 200);
508+
// tick 1: max(50, 30) = 50 — the wheel source is exhausted and contributes nothing.
509+
assert_eq!(tick_ms(1), 50);
510+
}
491511
}

0 commit comments

Comments
 (0)