Modify

Opened 6 years ago

Closed 6 years ago

#12335 closed defect (fixed)

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

Reported by: kolesar Owned by: team
Priority: normal Milestone: 16.02
Component: Core Version: tested
Keywords: template_report Cc:

Description

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)

Attachments (2)

ExtrudeAction.NullPointerException.fix.patch (338 bytes) - added by kolesar 6 years ago.
updateEnabledState.patch (6.4 KB) - added by kolesar 6 years ago.
patch for comment 5

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by kolesar

comment:1 Changed 6 years ago by Klumbumbus

Summary: ExtrudeAction causes NullPointerException if dual alignment setting is toggled without active layer[patch] ExtrudeAction causes NullPointerException if dual alignment setting is toggled without active layer

comment:2 Changed 6 years ago by bastiK

Milestone: 16.02

comment:3 Changed 6 years ago by 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.

comment:4 Changed 6 years ago by simon04

Resolution: fixed
Status: newclosed

In 9409/josm:

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

comment:5 Changed 6 years ago by kolesar

Resolution: fixed
Status: closedreopened

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.

Changed 6 years ago by kolesar

Attachment: updateEnabledState.patch added

patch for comment 5

comment:6 Changed 6 years ago by simon04

In 9444/josm:

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

comment:7 Changed 6 years ago by simon04

In 9445/josm:

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

comment:8 Changed 6 years ago by simon04

In 9446/josm:

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

comment:9 Changed 6 years ago by simon04

In 9447/josm:

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

comment:10 Changed 6 years ago by simon04

Resolution: fixed
Status: reopenedclosed
Summary: [patch] ExtrudeAction causes NullPointerException if dual alignment setting is toggled without active layerExtrudeAction causes NullPointerException if dual alignment setting is toggled without active layer, improve other enabled states

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() :)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.