[Edge] JavaDoc does not react to hovering/clicking

[fedejeanne]

See eclipse-jdt/eclipse.jdt.ui#2061
See eclipse-platform/eclipse.platform.swt#2072

Summary

• Hovering/clicking a JavaDoc should show the scrollbars and navigation controls.
• Using Tab or clicking in the lower part of the JavaDoc popup works as expected.

I assume this has something to do with an event not being properly triggered/handled by Edge.

[HeikoKlare]

Maybe related to eclipse-platform/eclipse.platform.swt#2072?

[fedejeanne]

It was the exact same issue. I closed it as a duplicate

[fedejeanne]

It was the exact same issue. I closed it as a duplicate

I reopened it and closed the JDT issue as a duplicate. It's easier to track this way.

[fedejeanne]

I marked it as a regression because, even though Edge didn't have that functionality before, IE did and therefore the consumer (JDT) experiences this issue as a regression. I also changed the priority to high so @amartya4256 can pick this as his next issue.

[amartya4256]

SWT cannot recieve the MouseUp or MouseDown event from the tooltip as the tooltip is running a webview instance (Edge browser) inside and when you click on the javadoc, the click is registered by Webview and not SWT. This is a problem since, there's no known way to directly get this mouseclick event out of Webview to SWT unlike IE. Hence, how we did it in eclipse-platform/eclipse.platform.swt#1551, we rather need to rely on mouseEnter and mouseMove instead.

The problem is that the toolTip is created using AdditionalInformationController where PopupCloser is resposible from replacing the tooltip control with the one with a scrollbar, but it doesn't handle any MouseEnter and MouseMove event unlike AbstarctHoverControlManager:Closer which is responsible for replacing the tooltip control in case of eclipse-platform/eclipse.platform.swt#1551. Hence, we need it to also register listeners for MouseEnter and MouseMove as done in the PR eclipse-platform/eclipse.platform.ui#2977. Although, thisaddition of extra events doesn't have any visual effects on other widgets (as far as I have tested), but it can indeed trigger replaceInformationControl on there 2 extra events too for all the controls using a popupCloser.

We can rather accept this solution as a feature to the widgets reacting to "mouseHover" for replacing the information control or figure out how edge can send a mouseUp event (which doesn't seem to be quite straight forward or maybe even not possible)

[fedejeanne]

Thank you for the details, Marty.

If it's too hard or even impossible to find a solution that only touches Edge (or at least only SWT) then I propose to simply wait until M1 and merge the one you already proposed: reacting/propagating the 2 new mouse events in PopupCloser (eclipse-platform/eclipse.platform.ui#2977).

[HeikoKlare]

If we could ensure that the browser mebedded into the PopupCloser has JavaScript enabled, there would also be the JavaScript solution of providing events from Edge to SWT.
Using the MouseEnter instead of the MouseUp event will also have UX impacts, so we need properly evaluate the effect and whether that is really what we want to have.

[amartya4256]

Found this bug report: https://bugs.eclipse.org/bugs/show_bug.cgi?id=228608
Where they decided to not enable JS for javadoc as it can be abused with malicious code as mentioned in the report. Also another example could be document.write("..."). Hence, just enabling JS with sratz's PR is not enough. We need to make sure, that injected scripts from the javadoc are not executed but our mouse tracker is still working and this is something we can do by escaping the script blocks.

Here's an example:

We do that by the following:
the method involved: CoreJavaDocAccess:getHtmlContentFromSource

protected String getHTMLContentFromSource(IJavaElement element) throws JavaModelException {
		IMember member;
		if (element instanceof ILocalVariable) {
			member= ((ILocalVariable) element).getDeclaringMember();
		} else if (element instanceof ITypeParameter) {
			member= ((ITypeParameter) element).getDeclaringMember();
		} else if (element instanceof IMember) {
			member= (IMember) element;
		} else {
			return null;
		}

		IBuffer buf= member.getOpenable().getBuffer();
		if (buf == null) {
			return null; // no source attachment found
		}

		ISourceRange javadocRange= member.getJavadocRange();
		if (javadocRange == null) {
			if (CoreJavadocContentAccessUtility.canInheritJavadoc(member)) {
				// Try to use the inheritDoc algorithm.
				String inheritedJavadoc= javadoc2HTML(member, element, "/***/"); //$NON-NLS-1$
				if (inheritedJavadoc != null && inheritedJavadoc.length() > 0) {
					return inheritedJavadoc;
				}
			}
			return getJavaFxPropertyDoc(member);
		}

// This is where we replace > and < with the html notations making sure that html-blocks will be displayed in the text format even when JS is enabled.
		String rawJavadoc= buf.getText(javadocRange.getOffset(), javadocRange.getLength()).replace("<", "&lt;").replace(">", "&gt;"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$

		return javadoc2HTML(member, element, rawJavadoc);
	}

After this, we can have JS enabled by default for BrowserInformationControl.

[HeikoKlare]

Thank you for the analysis!
You would probably need to limit the escape mechanism to <script> tags as otherwise all HTML intside Javadoc would be escaped, wouldn't it? But that should be possible.
But even then, I was not aware that JavaScript is disabled for those Javadoc controls. So the question is whether we then really want to change it for the sake of generating mouse events. I actually expected it to be enabled by default. Maybe we should then go with the already proposed solution based on MouseEnter/MouseExit but postpone it to M1 and then merge it early to see if there are complains throughout the 3 months of development.
What do you think?

[amartya4256]

Opened a discussion about it: eclipse-platform/eclipse.platform.ui#2996