DiffView: delegate mouse scroll events from lines bar to editor.

[mbien]

The LineNumbersActionsBar got a mind of its own and started scrolling on JDK 19+ on mouse wheel events without the rest of the diff view.

This simply delegates the events to the attached editor component, so that everything can scroll together.

fixes #6995

[mbien]

should we do this instead?

        linesActions.addMouseWheelListener(e -> {
            editorPane.dispatchEvent(e); // delegate mouse scroll events to editor
            e.consume();
        });

i am slightly worried that this is a JDK bug, since this did already stop the scrolling:

        linesActions.addMouseWheelListener(e -> {});

[eirikbakke]

Either is fine with me, as long as it works on all platforms.

[mbien]

I tested it on linux

[eirikbakke]

What does the component hierarchy look like here? Are the line numbers and text editor in two separate JScrollPane parents that are normally synced together somehow? If there's a JDK bug, what is the expected vs. actual behavior of the JScrollPane?

Though it might not be necessary to dig too deep here as long as the bug is fixed by the patch.

[mbien]

Are the line numbers and text editor in two separate JScrollPane parents that are normally synced together somehow?

right, otherwise the line numbers would scroll horizontally with the editor. They have their own scroll pane which scrolls only vertically and is linked to the editor they belong to. Then the two editors are linked in the diff view to one scroll bar, but mouse wheel allows to scroll them individually.

If there's a JDK bug, what is the expected vs. actual behavior of the JScrollPane?

for once the behavior should not change just by adding an empty mouse wheel listener. Which is why I thought about calling consume() after redispatch.

[neilcsmith-net]

Suspect caused by this change - https://bugs.openjdk.org/browse/JDK-6911375 / openjdk/jdk19u@832729b ?? We were perhaps reliant on a JDK bug that was fixed?

[eirikbakke]

I'm fine with whichever solution seems to work here. Ideally we'd test this it on all three platforms (Windows, Linux, and MacOS), though. I'm on Windows myself and can test there.

[eirikbakke]

Either is fine with me, as long as it works on all platforms.

[mbien]

Suspect caused by this change - https://bugs.openjdk.org/browse/JDK-6911375 / openjdk/jdk19u@832729b ?? We were perhaps reliant on a JDK bug that was fixed?

I don't want to confuse anyone, but the workaround for this bug is here:

netbeans/ide/diff/src/org/netbeans/modules/diff/builtin/visualizer/editable/DiffViewManager.java

Lines 111 to 121 in 4290a3d

	// show/hide left scrollbar depending on rights visibility
	if(rightContentPanel.getScrollPane().getVerticalScrollBar().isVisible()) {
	Dimension d = leftContentPanel.getScrollPane().getVerticalScrollBar().getSize();
	if(d.getHeight() > 0 && d.getWidth() > 0) {
	leftScrollBarPrefSize = d;
	}
	// The left vertical scroll bar must be there for mouse wheel to work correctly.
	// However it's not necessary to be seen (but must be visible so that the wheel will work).
	leftContentPanel.getScrollPane().getVerticalScrollBar().setPreferredSize(new Dimension(0, 0));
	leftContentPanel.getScrollPane().getVerticalScrollBar().setSize(new Dimension(0, 0));
	leftContentPanel.getScrollPane().getVerticalScrollBar().repaint();

this is a different scroll pane however.

the question is what the editor was supposed to do if you put the mouse on the line numbers and scroll. The regular editor does scroll, so I suppose it was the original intention too for the diff view - however even on JDK 11 this is not the case.

Fact is that adding an empty mouse wheel listener, already restores the old behavior on JDK 19+: nothing happens on scroll over the line number bar. I can't remember of an instance that adding an empty listener did ever change behavior in swing before!

This PR goes one little step further and delegates the scroll events to DecoratedEditorPane. The events propagate to DiffViewManager#stateChanged() (link above) which has all the scroll logic and in turn scrolls all linked components -> diff view behaves like the regular editor on all JDKs.

[neilcsmith-net]

The bug and fix I linked to in JDK 19 makes the scroll pane react to mouse wheel events even when no scroll bar is visible. Our code seems to rely on them never being there to filter out scrolling.

Fact is that adding an empty mouse wheel listener, already restores the old behavior on JDK 19+: nothing happens on scroll over the line number bar. I can't remember of an instance that adding an empty listener did ever change behavior in swing before!

It's not that unusual that adding a listener filters events being handed up to the ancestor. See the documentation for MouseWheelEvent -

 * MouseWheelEvents start delivery from the Component underneath the
 * mouse cursor.  If MouseWheelEvents are not enabled on the
 * Component, the event is delivered to the first ancestor
 * Container with MouseWheelEvents enabled.  This will usually be
 * a ScrollPane with wheel scrolling enabled.

The fix seems to make sense to me. And also that there is not a JDK bug involved.

[mbien]

Our code seems to rely on them never being there to filter out scrolling.

filtering out scrolling already is a bug since the regular editor behaves differently. This likely is just a coincidence, nobody wanted to actually ignore events there. Users only noticed when the numbers bar started to move that everything should have moved from the first place.

[neilcsmith-net]

Users noticed the numbers bar moving because the JDK bug is now fixed, so I assume filtering was desired unless the current behaviour should be considered correct! 😉

Anyway, the fix looks right for now. Ideally I guess it would be using https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/javax/swing/JScrollPane.html#setRowHeaderView(java.awt.Component) rather then two scroll panes, same as the main editor?

[mbien]

so I assume filtering was desired unless the current behaviour should be considered correct!

that is in fact the question. This PR assumes the behavior of the regular editor is correct, and that the diff view never was correct, the moving numbers bar only made the problem clearly visible.

If you want the JDK 11 behavior (of the diff view, which would be different to the regular editor) I can remove the event redispatch.

[neilcsmith-net]

I think we're meaning slightly different things by filtering there. Yes, keep the event dispatch. Closer to main editor UI the better IMO.