[webdriver] Add synchronization for wheel action#37260
[webdriver] Add synchronization for wheel action#37260xiaochengh merged 2 commits intoservo:mainfrom
Conversation
|
Nice change. Thank for updating |
Since we already add servo/components/script/script_thread.rs Lines 1160 to 1171 in a328aa4 The function is called on servo/components/script/script_thread.rs Line 1095 in a328aa4 |
22dd140 to
ee1b92b
Compare
Signed-off-by: PotatoCP <[email protected]>
|
Have updated expectation after #37385 |
| f32, | ||
| f64, | ||
| f64, | ||
| Option<WebDriverMessageId>, |
There was a problem hiding this comment.
I'm wondering why this is an option.
Is there any place where webdriver does not set message id on a MouseButton / MouseMove / WheelScroll action?
There was a problem hiding this comment.
1 command may trigger multiple events, for current solution, we only want to add webdriverid to the last event.
There was a problem hiding this comment.
Hmm, with some fresh thoughts, does that mean that if we want thing to be semantically correct, message ids should be set on something higher-level, instead of just actions? But we have the problem of matching webdriver actions and input events on the UA, which seems tough...
So I think this PR is fine on its own, but we should probably leave a note here (and probably also file an issue) about:
- When will this message ID be None and why
- Think about ways to improve the design
| let msg_id = if last { | ||
| self.current_action_id.get() | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
@xiaochengh Just like Trong said, we set the id to None if it's not the last action sent here.
| f32, | ||
| f64, | ||
| f64, | ||
| Option<WebDriverMessageId>, |
There was a problem hiding this comment.
Hmm, with some fresh thoughts, does that mean that if we want thing to be semantically correct, message ids should be set on something higher-level, instead of just actions? But we have the problem of matching webdriver actions and input events on the UA, which seems tough...
So I think this PR is fine on its own, but we should probably leave a note here (and probably also file an issue) about:
- When will this message ID be None and why
- Think about ways to improve the design
Signed-off-by: PotatoCP <[email protected]>
|
Hi @xiaochengh, I have added comments for the option. |
Implement action synchronization for key event. Previously only done for pointer #36932 and wheel #37260. --------- Signed-off-by: PotatoCP <[email protected]>
Implement action synchronization for wheel event. Previously only done for pointer here #36932.
Testing:
tests/wpt/meta/webdriver/tests/classic/perform_actions/wheel.py