ljader / redmine-mylyn-plugin

Eclipse Mylyn integration for Redmine ( Eclipse Connector Plugin )
71 stars 48 forks source link

Eclipse crashes when I click on the Estimated hours buttons #55

Open lsampaioweb opened 9 years ago

lsampaioweb commented 9 years ago

I am using Eclipse Luna. I installed the plugin from the zip file: net.sf.redmine_mylyn.p2repository-0.4.0-SNAPSHOT.zip. Whenever I click on one of two buttons to increase or decrease the amount of estimated hours, eclipse begins to "process" something and never stops.

Can you check this, please? I want to convince all my teammates to use your plugin, but if it continues to crash Eclipse, it will be difficult to convince them.

Thank you!

awilkins commented 9 years ago

Anything in the Eclipse error log? I was unable to reproduce this on my current build (which is from the tip of the source).

If you could also try checking out the source and building the plugin with

mvn clean package -Pplatform-luna

And installing the resulting package from the zipped up repository file in this folder..

net.sf.redmine_mylyn.p2repository/target

.. that would be helpful

lsampaioweb commented 9 years ago

Hi,

I downloaded the source, executed the maven command and installed from the generated zip file. After that I added some content to a dumb task. However, once again when I clicked on the Increase amount of estimated time, it hanged Eclipse until I had to shut it down. The entry on the log says "Unhandled event loop exception".

See attached screens. screen_01 screen_02 screen_03

awilkins commented 9 years ago

If you open the detail on that loop exception and provide the stack dump (as text, for preference) it will be useful.

lsampaioweb commented 9 years ago

java.lang.NumberFormatException: For input string: "1,25" at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:2043) at sun.misc.FloatingDecimal.parseFloat(FloatingDecimal.java:122) at java.lang.Float.parseFloat(Float.java:451) at java.lang.Float.valueOf(Float.java:416) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.setValue(EstimatedEditor.java:123) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.access$2(EstimatedEditor.java:121) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor$2.modifyText(EstimatedEditor.java:85) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:179) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4188) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1467) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1490) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1471) at org.eclipse.swt.widgets.Spinner.setSelection(Spinner.java:937) at org.eclipse.swt.widgets.Spinner.sendSelection(Spinner.java:741) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5469) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Widget.callSuper(Widget.java:221) at org.eclipse.swt.widgets.Widget.mouseDownSuper(Widget.java:1101) at org.eclipse.swt.widgets.Widget.mouseDown(Widget.java:1093) at org.eclipse.swt.widgets.Control.mouseDown(Control.java:2563) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5627) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Widget.callSuper(Widget.java:221) at org.eclipse.swt.widgets.Widget.windowSendEvent(Widget.java:2105) at org.eclipse.swt.widgets.Shell.windowSendEvent(Shell.java:2329) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5691) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Display.applicationSendEvent(Display.java:5128) at org.eclipse.swt.widgets.Display.applicationProc(Display.java:5277) at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method) at org.eclipse.swt.internal.cocoa.NSApplication.sendEvent(NSApplication.java:128) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3655) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1151) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1032) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:148) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:636) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:579) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:135) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:483) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465)

awilkins commented 9 years ago

This is a localization issue.

The validation for that field uses Float.valueOf() to parse the string entered. This method expects the anglocentric "." (dot) as a decimal separator. Your locale uses "," comma.

Have replaced instances where these strings are validated with .valueOf() with the default NumberFormat parser instead - this should use the default JVM locale.

Can you pull branch 55-parse-locale-formatted-floats and try that? If it works I'll merge this branch and it will be fixed in the next CI release.

awilkins commented 9 years ago

Looking further at the code, there are a number of places where "dot" assumptions are made (in regular expressions used to parse values, further use of methods that follow the same code path as .valueOf() like Float.parseFloat() which call FloatingDecimal.readJavaFormatString()

Clue's in the name readJavaFormatString() - this method is not for user-visible locale-formatted strings.

Patch made so far may fix this issue but I think it needs a bit more work see #56

awilkins commented 9 years ago

Have pushed another patch (same branch) that addresses all the locations I could find in the code that use FloatingDecimal.readJavaFormatString() to parse UI values.

This should fix locale problems where float values are using a comma for a decimal separator.

Could you pull and test this? The parts affected are

lsampaioweb commented 9 years ago

I did all you asked, now it is not hanging Eclipse anymore but there are still some errors.

java.lang.NumberFormatException: For input string: "2,75" at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:2043) at sun.misc.FloatingDecimal.parseFloat(FloatingDecimal.java:122) at java.lang.Float.parseFloat(Float.java:451) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.toSelectionValue(EstimatedEditor.java:150) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.access$1(EstimatedEditor.java:145) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor$1.attributeChanged(EstimatedEditor.java:51) at org.eclipse.mylyn.tasks.core.data.TaskDataModel$1.run(TaskDataModel.java:91) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.mylyn.tasks.core.data.TaskDataModel.attributeChanged(TaskDataModel.java:85) at org.eclipse.mylyn.tasks.ui.editors.AbstractAttributeEditor.attributeChanged(AbstractAttributeEditor.java:128) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.setValue(EstimatedEditor.java:141) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor.access$2(EstimatedEditor.java:124) at net.sf.redmine_mylyn.internal.ui.editor.EstimatedEditor$2.modifyText(EstimatedEditor.java:88) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:179) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4188) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1467) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1490) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1471) at org.eclipse.swt.widgets.Spinner.setSelection(Spinner.java:937) at org.eclipse.swt.widgets.Spinner.sendSelection(Spinner.java:741) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5469) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Widget.callSuper(Widget.java:221) at org.eclipse.swt.widgets.Widget.mouseDownSuper(Widget.java:1101) at org.eclipse.swt.widgets.Widget.mouseDown(Widget.java:1093) at org.eclipse.swt.widgets.Control.mouseDown(Control.java:2563) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5627) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Widget.callSuper(Widget.java:221) at org.eclipse.swt.widgets.Widget.windowSendEvent(Widget.java:2105) at org.eclipse.swt.widgets.Shell.windowSendEvent(Shell.java:2329) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5691) at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method) at org.eclipse.swt.widgets.Display.applicationSendEvent(Display.java:5128) at org.eclipse.swt.widgets.Display.applicationProc(Display.java:5277) at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method) at org.eclipse.swt.internal.cocoa.NSApplication.sendEvent(NSApplication.java:128) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3655) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1151) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1032) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:148) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:636) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:579) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:135) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:483) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465)

screen_04

awilkins commented 9 years ago

Yup, the latest revision on branch should fix that one - I reckon I've caught all of them now.

lsampaioweb commented 9 years ago

Almost there, but there are still some problems. The error only appeared when I pressed Submit.

java.lang.NumberFormatException: For input string: "5,75" at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:2043) at sun.misc.FloatingDecimal.parseFloat(FloatingDecimal.java:122) at java.lang.Float.parseFloat(Float.java:451) at java.lang.Float.(Float.java:532) at net.sf.redmine_mylyn.internal.core.IssueMapper.setProperty(IssueMapper.java:395) at net.sf.redmine_mylyn.internal.core.IssueMapper.createIssue(IssueMapper.java:165) at net.sf.redmine_mylyn.core.RedmineTaskDataHandler.postTaskData(RedmineTaskDataHandler.java:159) at org.eclipse.mylyn.internal.tasks.core.sync.SubmitTaskJob.run(SubmitTaskJob.java:101) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)

screen_05

awilkins commented 9 years ago

Pushed another patch

Hopefully that catches the last one of these problems ; new Float(String) also suffers from it, it seems.

lsampaioweb commented 9 years ago

Almost there!!! It does not show any error, but the value (Estimated Hours) is not correct. When I add the value 1 or 1,5 and submit, it changes to 10,00 or 15,00.

01 02 03 04

lsampaioweb commented 9 years ago

Hi there,

Any news ? The plugin is still showing some errors.

Thank you!

awilkins commented 9 years ago

Hi there, apologies for the somewhat spotty effort, maintaining this is not a part of my day job.

I know what the nature of the problem is, it's when the strings are read out again.. it's suffering from the same Locale based problems. Using a Java Scrapbook Page in Eclipse :

java.text.NumberFormat.getInstance(
    java.util.Locale.forLanguageTag("es")) // Spain
    .parse("1,5")
    .floatValue()

(float) 1.5

java.text.NumberFormat.getInstance() // using my default en-GB locale
    .parse("1,5")
    .floatValue()

(float) 15.0

Not sure where this happens in the code though. If it's happening in the core Mylyn code (and I suspect it is) that either means patching Mylyn or changing the approach I've been using here.

It's not the first instance of such a problem, see : https://bugs.eclipse.org/bugs/show_bug.cgi?id=256809

After doing a single save from Mylyn (and not reloading the ticket) what are the values stored in the Redmine instance that show up in the web UI?

lsampaioweb commented 9 years ago

On the plugin, I added the value 3 (Estimated Hours) and submitted. The plugin reloaded the content of the task and showed 30,00 as the value. However, the Redmine Web UI is correctly displaying 3.

So, what do you think I should do then ?

lsampaioweb commented 9 years ago

Any news or advice for me ?!

awilkins commented 9 years ago

What it really needs is consistent handling of how float values are converted into strings and vice-versa, all the way down the software stack.

The thing I've not had the time to do is pull the Mylyn sources and see how it handles that. If it makes the same presumption that floats are all in the standard Java format, then to fix this the way I've been going about it means patching Mylyn, which will be a more involved process.

Alternates...

To really fix this rigorously someone needs to work out what the behaviour of each component of the stack is...

.. and ensure they are all working together properly ; in general, I'd say that the only place the commas should show up are in strings presented to the user (this way you can control their conversion).

Don't have time to do this myself at present.