Minimap can cause NPE

[GeraldMit]

Summary:

Minimap assumes that the viewer is the Eclipse provided implementation and so can fail for other implementations, causing a NullPointerException. Need to validate the ITextViewer.getStyledText on usage incase it can dynamically exist or doesn't exist for that ITextViewer implementation instance at the time of the call, either find have an alternate supplier for the needed information from the ITextViewer object as other interfaces via other methods, or fail the activity for those cases ( or disallow minimap for those cases)

Issue:

java.lang.NullPointerException
at org.eclipse.ui.internal.views.minimap.MinimapWidget$EditorTracker.install(MinimapWidget.java:273)
at org.eclipse.ui.internal.views.minimap.MinimapWidget.install(MinimapWidget.java:508)
at org.eclipse.ui.internal.views.minimap.MinimapPage.createControl(MinimapPage.java:67)
at org.eclipse.ui.internal.views.minimap.MinimapView.doCreatePage(MinimapView.java:57)
at org.eclipse.ui.part.PageBookView.createPage(PageBookView.java:369)
at org.eclipse.ui.part.PageBookView.partActivated(PageBookView.java:696)
at org.eclipse.ui.part.PageBookView$1.partActivated(PageBookView.java:1006)
at org.eclipse.ui.internal.WorkbenchPage$3.run(WorkbenchPage.java:4880)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
at org.eclipse.ui.internal.WorkbenchPage.firePartActivated(WorkbenchPage.java:4877)
at org.eclipse.ui.internal.WorkbenchPage$E4PartListener.partActivated(WorkbenchPage.java:213)
at org.eclipse.e4.ui.internal.workbench.PartServiceImpl$2.run(PartServiceImpl.java:250)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.firePartActivated(PartServiceImpl.java:247)
at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:771)
at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:680)
at org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer.activate(AbstractPartRenderer.java:97)
at org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer.lambda$0(ContributedPartRenderer.java:63)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4579)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1524)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1547)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1532)
at org.eclipse.swt.widgets.Shell.setActiveControl(Shell.java:1637)
at org.eclipse.swt.widgets.Shell.setActiveControl(Shell.java:1600)
at org.eclipse.swt.widgets.Control.sendFocusEvent(Control.java:3447)
at org.eclipse.swt.widgets.Display.checkFocus(Display.java:703)
at org.eclipse.swt.widgets.Shell.makeFirstResponder(Shell.java:1319)
at org.eclipse.swt.widgets.Display.windowProc(Display.java:6458)
at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
at org.eclipse.swt.internal.cocoa.NSView.setHidden(NSView.java:264)
at org.eclipse.swt.widgets.Control.setVisible(Control.java:4437)
...

Investigation:

org.eclipse.jface.text.ITextViewer in the Eclipse API documentation for getTextWidget() that it may return null instead of a StyledWidget object

in minimap, variable fEditorViewer holds the ITextViewer reference and there are then references such as
StyledText editorTextWidget = fEditorViewer.getTextWidget();
where editorTextWidget assumes non-null in the MinimapWidget so then the next call happens without validation

which can cause if there is an ITextViewer loaded that returns a null for the getTextWidget() call.

Resolution plan: :

Step 1 is just to add checks to avoid NPE.
Step 2 is to test alternate means of getting the information where possible, which needs confirmation for all different findable cases via debugging to a 1-to-1 directly (via object storage) equivalent result.
Step 3 is to disengage Minimap for that viewer when a map is not possible.

Note: I am planning to submit a pull request for this correction unless someone beats me to it, but I am putting the issue out first so there is awareness as I want to thoroughly test my changes first and operating on a limited schedule.

[akurtakov]

Sorry for the long time to reply! Do you still plan to submit a patch ? I assume your patch is supposed to add checks to avoid NPE?

[GeraldMit]

I was still testing, and then was unable to work for a few days. I plan to continue work today.

[vogella]

@GeraldMit any upate for the PR?

[GeraldMit]

@vogella thank you for the reminder. I ran into some issues in step 2 when testing against broader usage, so I will check in step 1 when I get back in next week.