Bugzilla – Full Text Bug Listing
| Summary: | Rolling hours through Daylight Savings Time epochs fails; No ability for users to edit TimeZone field | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Scott Klein <scott.klein> | ||||||||||||||||||
| Component: | Nebula | Assignee: | Wim Jongman <wim.jongman> | ||||||||||||||||||
| Status: | CLOSED INVALID | QA Contact: | |||||||||||||||||||
| Severity: | normal | ||||||||||||||||||||
| Priority: | P3 | CC: | nebula-inbox, peter, wim.jongman, wim.jongman | ||||||||||||||||||
| Version: | unspecified | Flags: | wim.jongman:
iplog+
|
||||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||
| OS: | Windows XP | ||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Scott Klein
Created attachment 210529 [details]
Patch containing fixes/changes
Attached patch.
setTimeZone() methods should be reviewed. I am forcing the setting of one of the allowed time zones, when appropriate -- and *throwing an exception* when this is not the case. Developers should review this for conformity to standard.
There is one item I am unsure of, have not looked at yet: The use of a timezone in the DateFormat parts of the code. Need to make sure that when the Calendar object is formatted to a string (or vice-verse) that the format is using the currently active timezone -- since this can now be changed, the formatter needs to be re-instantiated or have the current timezone set to it.
I have looked at your patch. Quite a big one. It looks great. Thank you for this. All the current tests are passing when your patch is applied. To support the resolution of this bug I have changed the CDateTime example tab to enable the input of the TimeZone. I have already committed this change [1]. Please be advised that we do not longer use CVS but have switched to git. I have noticed that with your patch applied the movements between the fields in the pattern (with left and right cursor keys) the example will hang Eclipse for some reason. Can you take a look if this is happening to you too? [1] http://git.eclipse.org/c/nebula/org.eclipse.nebula.git/commit/?id=3616c6d32d3cae32d82ff9757b3fd2286a83d63a
>
> setTimeZone() methods should be reviewed. I am forcing the setting of one of
> the allowed time zones, when appropriate -- and *throwing an exception* when
> this is not the case. Developers should review this for conformity to standard.
You should evaluate the possible impact this can have on other people. If you think their code would suddenly break when installing a new version of CDateTime then throwing an exception is a no-go.
As an alternative you could consider introducing new methods setTimeZoneChecked() that will throw the runtime exception.
Regarding the thrown exceptions, I have reverted the setTimeZone() methods back to their originals. I then altered my fieldRoll() code, where the TimeZone field is "spun", to revert back to the first TimeZone in the user defined array if the TimeZone (being switched out of) does not exist in that user defined array. This should relieve any concerns of impact on backwards compatibility. Though it does introduce the possibility that the first TimeZone displayed to the user may never be spun back to once it has been changed. For example, if the timezone is initially set to "GMT" and the allowed TimeZones are "PST, CST and EST". The user will see GMT (and even be able to enter their time in that TimeZone), however once they spin away from GMT they will never see GMT re-appear. Patch update pending testing and test cases OK. Took me a while to get this git/maven thing setup and figure out the initial testing framework set up. Now firing ahead and just have a few more tests to write. (In reply to comment #2) > All the current tests are passing when your patch is applied. > > Please be advised that we do not longer use CVS but have switched to git. > > I have noticed that with your patch applied the movements between the fields in > the pattern (with left and right cursor keys) the example will hang Eclipse for > some reason. Can you take a look if this is happening to you too? I just finished running all of the existing tests and they are passing just fine. I did notice that when I was testing under my CVS checkout the spinner tests were *not* passing. They *are* passing in the git checkout code. Created attachment 210701 [details]
Test Case for this bug (requires update to CdtTester)
Added test case for this bug. This required some additions to the test proxy interface.
Created attachment 210702 [details]
CdtTester.java
OK. So being a n00b to git I can't figure out how to generate a patch (if you can't right click to create it I am lost!). Anyway, this is the updated test proxy class. I will look again tomorrow, but wanted to get something up.
Created attachment 210703 [details]
CDateTime.java - full file
(In reply to comment #7) If you use egit then it is just right-click and Team/Create Patch.. from the package explorer. This looks good. We still need a few more things: 1. A snippet for this cool new function. 2. An update of the examples view 3. Passing Tests on Linux and Mac I will take care of 2 after you have supplied 1. I can test on Mac are you able to do so on Linux? Created attachment 210750 [details]
CDTSnippet09.java
Attached snippet code
Scott do you agree that your additions are made available under the terms of the Eclipse Public License v1.0 http://www.eclipse.org/legal/epl-v10.html (In reply to comment #12) > Scott do you agree that your additions are made available under the terms of > the Eclipse Public License v1.0 > > http://www.eclipse.org/legal/epl-v10.html Yes, I agree that my additions will be made available under the terms of the Eclipse Public License v1.0 pushed to master. should be available from the incubation build in a few minutes Created attachment 210767 [details]
mylyn/context/zip
Thanks for the hard work Scott. Would you mind taking a look at the example view? I cannot get it to work as your snippet. Even though I added a bunch of timezones, it still only lets me roll thru GMT and CET even though these are not in the list of timezones I add to the widget. (In reply to comment #16) > Thanks for the hard work Scott. Would you mind taking a look at the example > view? I cannot get it to work as your snippet. Even though I added a bunch of > timezones, it still only lets me roll thru GMT and CET even though these are > not in the list of timezones I add to the widget. I am showing the same thing. rather than "CST,EST,ART,EET,AKST" I get CST,GMT on down arrow and just GMT on up arrow (or the other way, can't remember which arrow did what now). Anyway, looking into it now. hey Wim(In reply to comment #16) > Thanks for the hard work Scott. Would you mind taking a look at the example > view? I cannot get it to work as your snippet. Even though I added a bunch of > timezones, it still only lets me roll thru GMT and CET even though these are > not in the list of timezones I add to the widget. hey Wim, just add a .trim() call after you split the time zone text in the setPattern() method. for (int i = 0; i < zones.length; i++) { tZones[i] = TimeZone.getTimeZone(zones[i].trim()); } Thanks! When I input: GMT, Etc/GMT+1, Etc/GMT+2 it rolls through these values: 01/10/2012 08:26.05 GMT 01/10/2012 07:26.05 GMT-01:00 01/10/2012 06:26.05 GMT-02:00 The calculation seams okay judging from this website [1]. What it displays in the roll-through does not visually match the input. [1] http://www.gsp.com/support/virtual/admin/unix/tz/gmt/ Snippet and testcase pushed to master. Created attachment 210838 [details]
mylyn/context/zip
(In reply to comment #20) > When I input: > > GMT, Etc/GMT+1, Etc/GMT+2 > > it rolls through these values: > > 01/10/2012 08:26.05 GMT > 01/10/2012 07:26.05 GMT-01:00 > 01/10/2012 06:26.05 GMT-02:00 > > The calculation seams okay judging from this website [1]. What it displays in > the roll-through does not visually match the input. > > [1] http://www.gsp.com/support/virtual/admin/unix/tz/gmt/ You mean the time zone ID of "Etc/GMT+1" does not seem consistent with "GMT-01:00" I, by no means, pretend to know or understand the arcane arts of time zone management -- all I can really do is say "i guess that's the way it is supposed to be" because all of the "Etc/GMT%" time zones have an ID that contains the opposite sign of their actual offset. Here is the "-1" item (ID, Display, Offset, Example) Etc/GMT-1 GMT+01:00 1 2012-02-03 19:32:38 GMT+01:00 Not very satisfying, but couldn't find any useful info on these time zones. (In reply to comment #23) > Not very satisfying, but couldn't find any useful info on these time zones. http://www.murga-linux.com/puppy/viewtopic.php?t=72294&start=15&sid=ea421b58b9b5f265264bbae82973a1f6 You could accept display names (rather than IDs) in that text control, but this would be a lot more painful to write (and use). Then your input/output would be consistent. Created attachment 210865 [details]
CDateTimeExampleTab.java
Wim, take a look at this little refactor of the example tab. See if you like any part of it.
I have added a button next to the "Set Format" button which triggers a popup list to select the time zones. Once selected, I set them into the (now hidden) time zone text control for access on format setting. Just be sure to press "Set Format" once you have selected your time zones.
Apart from the last example tab (it is not quite there yet) is the patch working for you if you get it from our p2 site? Scott I am seeing NPE's if I hit the Set format button more then once. java.lang.NullPointerException at java.util.GregorianCalendar.computeFields(Unknown Source) at java.util.GregorianCalendar.computeFields(Unknown Source) at java.util.Calendar.setTimeInMillis(Unknown Source) at java.util.Calendar.setTime(Unknown Source) at java.text.SimpleDateFormat.format(Unknown Source) at java.text.SimpleDateFormat.formatToCharacterIterator(Unknown Source) at org.eclipse.nebula.widgets.cdatetime.CDateTime.updateFields(CDateTime.java:1825) at org.eclipse.nebula.widgets.cdatetime.CDateTime.setPattern(CDateTime.java:1651) at org.eclipse.nebula.widgets.cdatetime.example.CDateTimeExampleTab.setPattern(CDateTimeExampleTab.java:361) at org.eclipse.nebula.widgets.cdatetime.example.CDateTimeExampleTab$10.widgetSelected(CDateTimeExampleTab.java:265) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:240) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4165) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3754) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2696) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2660) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2494) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:674) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:667) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:123) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:344) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:622) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:577) at org.eclipse.equinox.launcher.Main.run(Main.java:1410) at org.eclipse.equinox.launcher.Main.main(Main.java:1386) This bug does not have a target milestone assigned and is automatically closed as part of the 2.3.0 release cleanup. It could be that this bug is accidentally closed for which we apologize. If this bug is still relevant, please re-open and set a target milestone. |