Edge: Implement mouse-related event listener support #2097

sratz · 2025-05-05T14:24:33Z

MouseListener
- up
- down
- doubleClick
MouseMoveListener
- move
MouseTrackListener
- exit
- enter
MouseWheelListener
- scroll
DragDetectListener
- dragDetected

The implementation is JavaScript-based by attaching listeners to the DOM for all relevant events and forwarding them to the WebView via window.chrome.webview.postMessage().

The event handling is analogous to IE's
IE#handleDOMEvent(org.eclipse.swt.ole.win32.OleEvent).

Note: Since the implementation is JavaScript-based, this requires JavaScript to be enabled on the Browser instance.

When JavaScript is disabled, we keep using the timer-based fallback implementation introduced in #1551.
As JavaScript is only truly enabled after navigation has finished, we also keep using this workaround when the Browser is first instantiated and before the first page has loaded.

This change also fixes the jsEnabled flag lifecycle by updating it in handleNavigationCompleted(), same as in IE.

This resolves #2164 as long as JavaScript is enabled.

github-actions · 2025-05-05T14:25:21Z

Test Results

118 files ±0 118 suites ±0 10m 18s ⏱️ + 1m 6s
4 432 tests ±0 4 412 ✅ +1 17 💤 ±0 3 ❌ - 1
298 runs ±0 291 ✅ +1 4 💤 ±0 3 ❌ - 1

For more details on these failures, see this check.

Results for commit ae05ca9. ± Comparison against base commit 7d4190a.

♻️ This comment has been updated with latest results.

HeikoKlare · 2025-05-05T14:31:45Z

May this fix ...?

[Regression] Can't click on JavaDoc popup, resize or scroll #2072

sratz · 2025-05-05T15:11:36Z

May this fix ...?

[Regression] Can't click on JavaDoc popup, resize or scroll #2072

Not sure.

I just remember that we've been down this road before.

The problem is that if we disable JavaScript via ICoreWebView2Settings.put_IsScriptEnabled(boolean), this will also prevent those listeners from running :(

akurtakov · 2025-07-29T14:26:29Z

Is this still being considered?

HeikoKlare · 2025-07-29T14:30:03Z

Is this still being considered?

Yes, we still have #2164 and just had an offline talk about how we want to proceed with this a few weeks ago. I expect that we proceed with this or have something else supercede it in the next weeks.

sratz · 2025-08-12T11:15:31Z

Is this still being considered?

Yes, we still have #2164 and just had an offline talk about how we want to proceed with this a few weeks ago. I expect that we proceed with this or have something else supercede it in the next weeks.

As discussed offline, I changed the implementation to keep the fallback in place

before first page load (where we don't have any javascript yet)
when javascript is disabled

sratz · 2025-09-03T07:42:54Z

This should be ready to go now.

sratz · 2025-09-18T09:11:23Z

@HeikoKlare Should we get this into M1?

HeikoKlare

Yes, I would be in favor of bringing this into M1. I currently don't have time for an in-depth review, but I went through the code once and it looks fine to me. Still some testing would probably be good. In particular, testing with different HiDPI scenarios (monitor-specific scaling enabled/disabled with different zooms of multiple monitors) would make sense to me, as the coordinate calculation could easily break depending on the used coordinate system. @amartya4256 could you do that?
In any case, we will of course get good implicit testing when merging this for M1. But at least the non-monitor-specific-scaling case should be explicitly captured, as it's not the default anymore and thus potential issues with it will easily not be noticed otherwise.

amartya4256 · 2025-09-18T16:34:42Z

bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java

+	private static final String EVENT_DOUBLECLICK = "dblclick"; //$NON-NLS-1$
+	private static final String EVENT_MOUSEWHEEL = "wheel"; //$NON-NLS-1$
+	private static final Set<String> MOUSE_RELATED_DOM_EVENTS = Set.of( //
+			EVENT_MOUSEDOWN, EVENT_MOUSEUP, //


Can we get rid of the trailing //? what is the purpose of them here?

I wanted to group the different semantics / pairs per line

Yes I often do that too to make details more readable.

Okay good to know :)

amartya4256 · 2025-09-19T10:16:30Z

bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java

+ * Insipired by the mouse-event related parts of
+ * {@link IE#handleDOMEvent(org.eclipse.swt.ole.win32.OleEvent)}
+ */
+private void handleMouseRelatedDomEvent(MouseRelatedDomEvent domEvent) {


Since this method can take care of mouse related events, shall we get rid of handleMouseMovement and implement it here instead? Then we can also handle them right here.

Not sure I understand.

handleMouseMovement() is the fallback handling when javascript is disabled which produces artifical events.

handleMouseRelatedDomEvent(MouseRelatedDomEvent) is the logic that is being invoked from within javascript.

Getting rid of handleMouseMovement would mean we'd have to produce artificial DOM events inside the scheduleMouseMovementHandlingIfNeeded() lambda just to unpack them again in handleMouseRelatedDomEvent.

I don't think this makes sense.

But we should probably rename

scheduleMouseMovementHandlingIfNeeded() -> scheduleFallbackMouseMovementHandlingIfNeeded()

handleMouseMovement() -> handleFallbackMouseMovement()

for clarity.

Oh I completely misunderstood. The name changes are fine. I would like to propose that we change the field needsMouseMovementFallback to a private method. I'll comment where its intended.

amartya4256 · 2025-09-19T11:03:53Z

bundles/org.eclipse.swt/Eclipse SWT Browser/win32/org/eclipse/swt/browser/Edge.java

 	}
+	if (top) {
+		jsEnabled = jsEnabledOnNextPage;
+		needsMouseMovementFallback = !jsEnabled;


How about we remove it and create a private method needsMouseMovementFallback() which always returns !jsEnabled. Then we do not rely on jsEnabled being set in handleNavigationCompleted since it is a package protected field.

The problem is that the initial value of jsEnabled is true, but the initial value of needsMouseMovementFallback also must be true, because we need the fallback at least once until the first website has successfully loaded and we then receive javascript events.

That's why I introduced this separate field.

I'll add a comment to the field explaining this subtlety

Alternatively, we could introduce a new field atLeastOnceTopNavigationCompleted, and then have something like

private boolean atLeastOnceTopNavigationCompleted; boolean needsMouseMovementFallback() { return !atLeastOnceTopNavigationCompleted || !jsEnabled; }

Ahhh, since they are counter-opposite of each other. But this still puts needsMouseMovementFallback at the risk of being inconsistent. What if jsEnabled is updated from outside of Edge class. I know its not ideal but still possible.

jsEnabled is never changed directly from the outsite.

setJavaScriptEnabled(boolean) updates jsEnabledOnNextPage and only in handleNavigationCompleted() are jsEnabled + needsMouseMovementFallback then updated.

Yes I agree its not set from outside but since it is accessible from outside, I meant from access modifier perspective.
We should use a getter instead. I like your proposal above more than the current state for safety.

amartya4256

I tested the code out. Functionality-wise the code uses the new handler when javascript is enabled to capture cursor movements. Although, I think the needsMouseMovementFallback being assigned in the method handleNavigationCompleted is a bit iffy. I am just being nitpicky here. I approve this PR.

sratz · 2025-09-23T13:49:06Z

I tested the code out. Functionality-wise the code uses the new handler when javascript is enabled to capture cursor movements.

Thanks for testing and reviewing!

Although, I think the needsMouseMovementFallback being assigned in the method handleNavigationCompleted is a bit iffy. I am just being nitpicky here. I approve this PR.

I fully agree. I was also struggling and refactoring this part over several iterations - and it always felt off. Even when refactoring it into a helper method, the place where this condition effectively must change is still in handleNavigationCompleted, as there the jsEnabledFlag is toggled. So I thought it's at least consistent to have 2 flags and not 1 flag + 1 helper method (which uses the first flag as well).

sratz · 2025-09-23T15:05:55Z

I've given it some more thought and I think the flag atLeastOneTopNavigationCompleted isn't that bad actually. It expresses better why we need the fallback handling.

- MouseListener up down doubleClick - MouseMoveListener move - MouseTrackListener exit enter - MouseWheelListener scroll - DragDetectListener dragDetected The implementation is JavaScript-based by attaching listeners to the DOM for all relevant events and forwarding them to the WebView via window.chrome.webview.postMessage(). The event handling is analogous to IE's IE#handleDOMEvent(org.eclipse.swt.ole.win32.OleEvent). Note: Since the implementation is JavaScript-based, this requires JavaScript to be enabled on the Browser instance. When JavaScript is disabled, we keep using the timer-based fallback implementation introduced in eclipse-platform#1551. As JavaScript is only truly enabled *after* navigation has finished, we also keep using this workaround when the Browser is first instantiated and before the first page has loaded. This change also fixes the jsEnabled flag lifecycle by updating it in handleNavigationCompleted(), same as in IE. This resolves eclipse-platform#2164 as long as JavaScript is enabled.

amartya4256 · 2025-09-24T10:38:36Z

I've given it some more thought and I think the flag atLeastOneTopNavigationCompleted isn't that bad actually. It expresses better why we need the fallback handling.

Yes, I think it is very descriptive. Let's go with it.

sratz · 2025-09-24T17:00:06Z

Test failures on Windows are unrelated.

sratz added the edge Edge Browser label May 5, 2025

sratz marked this pull request as draft May 5, 2025 15:47

fedejeanne mentioned this pull request May 20, 2025

Edge: addMouseWheelListener and addMouseListener have no effect #2164

Closed

fedejeanne linked an issue May 21, 2025 that may be closed by this pull request

Edge: addMouseWheelListener and addMouseListener have no effect #2164

Closed

sratz force-pushed the edge_mouseevents branch from 0d75c70 to b17cd3f Compare August 12, 2025 11:13

sratz force-pushed the edge_mouseevents branch from b17cd3f to c8ac9dd Compare August 12, 2025 11:32

sratz force-pushed the edge_mouseevents branch from c8ac9dd to 1a57180 Compare September 3, 2025 07:41

sratz marked this pull request as ready for review September 3, 2025 07:41

HeikoKlare reviewed Sep 18, 2025

View reviewed changes

amartya4256 reviewed Sep 18, 2025

View reviewed changes

amartya4256 reviewed Sep 19, 2025

View reviewed changes

sratz force-pushed the edge_mouseevents branch from 1a57180 to 54645b9 Compare September 19, 2025 13:25

amartya4256 approved these changes Sep 23, 2025

View reviewed changes

sratz force-pushed the edge_mouseevents branch from 54645b9 to ae05ca9 Compare September 23, 2025 15:07

sratz merged commit cbf7f7a into eclipse-platform:master Sep 24, 2025
15 of 17 checks passed

sratz deleted the edge_mouseevents branch September 24, 2025 17:00

sratz added this to the 4.38 M1 milestone Sep 24, 2025

Edge: Implement mouse-related event listener support #2097

Edge: Implement mouse-related event listener support #2097

Conversation

sratz commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

HeikoKlare commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sratz commented May 5, 2025

Uh oh!

akurtakov commented Jul 29, 2025

Uh oh!

HeikoKlare commented Jul 29, 2025

Uh oh!

sratz commented Aug 12, 2025

Uh oh!

sratz commented Sep 3, 2025

Uh oh!

sratz commented Sep 18, 2025

Uh oh!

HeikoKlare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amartya4256 Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sratz Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sratz Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amartya4256 Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amartya4256 left a comment

Choose a reason for hiding this comment

Uh oh!

sratz commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sratz commented Sep 23, 2025

Uh oh!

amartya4256 commented Sep 24, 2025

Uh oh!

sratz commented Sep 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

sratz commented May 5, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading

HeikoKlare commented May 5, 2025 •

edited

Loading

amartya4256 Sep 19, 2025 •

edited

Loading

sratz Sep 19, 2025 •

edited

Loading

sratz Sep 19, 2025 •

edited

Loading

amartya4256 Sep 19, 2025 •

edited

Loading

sratz commented Sep 23, 2025 •

edited

Loading