ExtrudeAction causes NullPointerException if dual alignment setting is toggled without active layer, improve other enabled states

[kolesar]

What steps will reproduce the problem?

1. Create a new empty layer in JOSM. (Ctrl+N)
2. Delete this layer.
3. Click Edit / Dual alignment.

What is the expected result?

Dual alignment checkbox changes.

What happens instead?

NullPointerException

Please provide any additional information below. Attach a screenshot if possible.

Line #366 should be deleted from core/src/org/openstreetmap/josm/actions/mapmode/ExtrudeAction.orig.java (tested, attached patch). Call updateStatusLine() on toggle is not needed, even confuses user because it says in status line that Extrude (with or without dual alignment) is the active tool while it is not.

"Drag a way segment to make a rectangle. Ctrl-drag to move a segment along its normal, Alt-drag to create a new rectangle, double click to add a new node. Dual alignment active."

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2016-01-06 17:30:31 +0100 (Wed, 06 Jan 2016)
Build-Date:2016-01-06 16:32:31
Revision:9329
Relative:URL: ^/trunk

Identification: JOSM/1.5 (9329 en) Linux Ubuntu 15.10
Memory Usage: 281 MB / 672 MB (204 MB allocated, but free)
Java version: 1.8.0_66, Oracle Corporation, Java HotSpot(TM) Server VM


Last errors/warnings:
- E: java.lang.NullPointerException
- W: java.net.UnknownHostException: josm.openstreetmap.de
- W: Unable to detect latest version of JOSM: josm.openstreetmap.de

java.lang.NullPointerException
	at org.openstreetmap.josm.actions.mapmode.MapMode.updateStatusLine(MapMode.java:78)
	at org.openstreetmap.josm.actions.mapmode.ExtrudeAction.toggleDualAlign(ExtrudeAction.java:366)
	at org.openstreetmap.josm.actions.mapmode.ExtrudeAction.access$000(ExtrudeAction.java:62)
	at org.openstreetmap.josm.actions.mapmode.ExtrudeAction$DualAlignChangeAction.actionPerformed(ExtrudeAction.java:199)
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
	at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2348)
	at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
	at javax.swing.JToggleButton$ToggleButtonModel.setPressed(JToggleButton.java:308)
	at javax.swing.AbstractButton.doClick(AbstractButton.java:376)
	at javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:833)
	at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(BasicMenuItemUI.java:877)
	at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:289)
	at java.awt.Component.processMouseEvent(Component.java:6535)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
	at java.awt.Component.processEvent(Component.java:6300)
	at java.awt.Container.processEvent(Container.java:2236)
	at java.awt.Component.dispatchEventImpl(Component.java:4891)
	at java.awt.Container.dispatchEventImpl(Container.java:2294)
	at java.awt.Component.dispatchEvent(Component.java:4713)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4888)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4525)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4466)
	at java.awt.Container.dispatchEventImpl(Container.java:2280)
	at java.awt.Window.dispatchEventImpl(Window.java:2750)
	at java.awt.Component.dispatchEvent(Component.java:4713)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
	at java.awt.EventQueue$4.run(EventQueue.java:731)
	at java.awt.EventQueue$4.run(EventQueue.java:729)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

[simon04]

Angle snapping, Dual alignment make no sense when there is no edit layer available. Thus, disable them.

attachment:ExtrudeAction.NullPointerException.fix.patch​ disables that status update also for valid usages.

[simon04]

In 9409/josm:

fix #12335 - Disable "Angle snapping", "Dual alignment" when no edit layer is present, fixes NPE

[kolesar]

Thank you for your quick commit. There is still need to disable updating status line after toggling. See the following steps:

1. Create a new empty layer in JOSM. (Ctrl+N)
2. Create a new Imagery layer (Imagery / OpenStreetMap)
3. Make Imagery layer active
4. Toggle Edit / Dual alignment

You will see message from Extrude tool in the status bar. This message is false there, Extrude tool is not active on an Imagery layer.

Possible solutions:

1. Disable updating status line for mapmodes that are not currently selected.
2. Disable toggling menu entries of mapmodes when they are not currently selected.
3. Disable toggling menu entries of mapmodes when active layer is not a data layer.
4. Disable toggling menu entries of mapmodes when there is no data layer at all.
5. Disable toggling menu entries of mapmodes when they are not currently selected or there is no active layer or active layer is not a data layer.

In [9409] you have choosen (d). I would recommend (e). Implementation is not more complicated than yours:

  setEnabled(Main.map != null && Main.map.mapMode instanceof ExtrudeAction);

Edit / Search for objects by preset (Shift+F3) was enabled always while Search (Crtl+F) disabled itself when getEditLayer() was null, fixed in patch. Also added three dots to menu entry because it pops up a dialog.

OrthogonalizeAction had a comment above updateEnabledState():

  /**
   * Don't check, if the current selection is suited for orthogonalization.
   * Instead, show a usage dialog, that explains, why it cannot be done.
   */

I can understand author but most other actions behave the opposite way: they disable their menu items when getCurrentDataSet() is null or getSelected().isEmpty(). Therefore I have modified OrthogonalizeAction to be similar to other tools.

I have also modified updateEnabledState() of AutoScaleAction. "Zoom to layer" and "Zoom to download" were active when they cannot do anything. Added call to updateEnabledState() from AutoScaleAction's MapFrameListener to disable "Zoom to previous" and "Zoom to next" after last layer was deleted.

[kolesar]

patch for comment 5

[simon04]

In 9444/josm:

see #12335 - Enable "Angle snapping", "Dual alignment" only in corresponding modes (patch by kolesar)

[simon04]

In 9445/josm:

see #12335 - Disable "Search for objects by preset" w/o edit layer (patch by kolesar)

[simon04]

In 9446/josm:

see #12335 - Make "Orthogonalize Shape" enabled state consistent w/ other actions (patch by kolesar)

[simon04]

In 9447/josm:

see #12335 - Improve "Zoom to layer" and "Zoom to download" enabled states (patch by kolesar)

[simon04]

Thank you a lot for your contributions. I split up your patch into several commits to simplify the understanding why a specific line has been changed. And, I changed size()>0 to !isEmpty() :)