test: Use real mouse events in tests on Firefox #20889
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,7 @@ | |
| "ArrowDown": "\uE015", | ||
| "Insert": "\uE016", | ||
| "Delete": "\uE017", | ||
| "Meta": "\uE03D", | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -502,7 +503,62 @@ def mouse( | |
| :param metaKey: press the meta key | ||
| """ | ||
| self.wait_visible(selector) | ||
|
|
||
| # HACK: Chromium clicks don't work with iframes; use our old "synthesize MouseEvent" approach | ||
| # https://issues.chromium.org/issues/359616812 | ||
| # TODO: x and y are not currently implemented: webdriver (0, 0) is the element's center, not top left corner | ||
| if self.browser == "chromium" or x != 0 or y != 0: | ||
| self.call_js_func('ph_mouse', selector, event, x, y, btn, ctrlKey, shiftKey, altKey, metaKey) | ||
| return | ||
|
|
||
| # For Firefox and regular clicks, use the BiDi API, which is more realistic -- it doesn't | ||
| # sidestep the browser | ||
| element = self.call_js_func('ph_find_scroll_into_view', selector) | ||
|
There was a problem hiding this comment. So this gives us a 200ms penalty per click, isn't that quite noticeable when running the tests? There was a problem hiding this comment. Also we don't always have to scroll into view I assume? If the element is visible we don't have to scroll do we? There was a problem hiding this comment. Right -- most tests should be fine without scrolling, but some (like the services or logs page) need it. There is https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoViewIfNeeded , but it's only implemented in Chromium. But this would not actually help with having to wait for it. The extra time isn't noticeable when you look at how long a test runs. Most of the BiDi churn is actually waiting for stuff (and our I'm not proud of this, but I tried different approaches and nothing else was reliable 😢 There was a problem hiding this comment. scrollIntoViewIfneeded sounds nice, should we add a future note about using that? There was a problem hiding this comment. We already use it for pixel tests. But it would only give us a minor optimization (page jumping around less if you look at it in an interactive browser). The problem is knowing when it's done, and *IfNeeded() doesn't help us there in the slightest. What I'd need would be some query if the element is currently visible. I didn't find one a few days ago, but I can do some more experiments and look harder. Then we can do this conditionally. There was a problem hiding this comment. I experimented with
There was a problem hiding this comment. There was a problem hiding this comment. Some failures are shallow (and I'll commit the fixes anyway), but this failure shows why this bounding rect approach doesn't work: Here the element is in a scrollable dialog pane, and hidden by defaullt. You have to scroll the dialog, not the page, to make it visible. This failure is robust (can be run interactively, with sleeps and sits). There was a problem hiding this comment. I didn't find a way around that, and scrollIntoViewIfNeeded() wouldn't help us either as it does not have a return value. So this was an interesting experiment, but failed 😢 Back to the original. |
||
|
|
||
| actions = [{"type": "pointerMove", "x": 0, "y": 0, "origin": {"type": "element", "element": element}}] | ||
| down = {"type": "pointerDown", "button": btn} | ||
| up = {"type": "pointerUp", "button": btn} | ||
| if event == "click": | ||
| actions.extend([down, up]) | ||
| elif event == "dblclick": | ||
| actions.extend([down, up, down, up]) | ||
| elif event == "mouseenter": | ||
| actions.insert(0, {"type": "pointerMove", "x": 0, "y": 0, "origin": "viewport"}) | ||
| elif event == "mouseleave": | ||
| actions.append({"type": "pointerMove", "x": 0, "y": 0, "origin": "viewport"}) | ||
| else: | ||
| raise NotImplementedError(f"unknown event {event}") | ||
|
|
||
| # modifier keys | ||
| ev_id = f"pointer-{self.driver.last_id}" | ||
| keys_pre = [] | ||
| keys_post = [] | ||
|
|
||
| def key(type_: str, name: str) -> JsonObject: | ||
| return {"type": "key", "id": ev_id + type_, "actions": [{"type": type_, "value": WEBDRIVER_KEYS[name]}]} | ||
|
|
||
| if altKey: | ||
| keys_pre.append(key("keyDown", "Alt")) | ||
| keys_post.append(key("keyUp", "Alt")) | ||
| if ctrlKey: | ||
| keys_pre.append(key("keyDown", "Control")) | ||
| keys_post.append(key("keyUp", "Control")) | ||
| if shiftKey: | ||
| keys_pre.append(key("keyDown", "Shift")) | ||
| keys_post.append(key("keyUp", "Shift")) | ||
| if metaKey: | ||
| keys_pre.append(key("keyDown", "Meta")) | ||
| keys_post.append(key("keyUp", "Meta")) | ||
|
|
||
| # the actual mouse event | ||
| actions = [{ | ||
| "id": ev_id, | ||
| "type": "pointer", | ||
| "parameters": {"pointerType": "mouse"}, | ||
| "actions": actions, | ||
| }] | ||
|
|
||
| self.bidi("input.performActions", context=self.driver.context, actions=keys_pre + actions + keys_post) | ||
|
|
||
| def click(self, selector: str) -> None: | ||
| """Click on a ui element | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda lean to making the X/Y API more logical in the future:
So this usually relates to the thing you want to click. So once the chromium bug is resolved I'd be for changing this to the BiDi API as well and making it the x/y coordinate relative to the clicked element (this feels very logical).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The x/y coords are relative to the clicked element of course -- but for MouseEvent, i.e. via
ph_mouse(), (0, 0) is the top left, while with webdriver (0, 0) is the element's center.Once we can run chromium with BiDi as well, then indeed I'm all for just redefining x/y here, and adjusting tests to specify the delta relative to the center. That's just impractical with this "half pm_mouse/half BiDi" state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we are on the same page 👍