Community
Participate
Working Groups
Using Mylyn from HEAD of today * open bugzilla task * change priority * the priority editor still shows the same value as before * submit works as expected
Looks like a regression that was introduced by the changes for bug 343005.
I am not seeing this problem.
Sam, I see this. This happens if you change the value from the editor in the summary (the icon stays the same)
After updating again, I now see the problem. I'll look into it now.
Steffen, this is a result of the change you made on the parent bug: (In reply to comment #10) > I have committed a fix to ignores model events that were generated by the > attribute editor itself. I deliberately had put that mechanism in the subclasses that needed it rather than in AbstractAttributeEditor because I was afraid of introducing bugs like this. Maybe it would be better to undo your change and add it to whichever subclass I apparently missed. For which attribute editor did you see the problem?
The problem is that the original patch on bug 343005 changed the behavior of PriortyAttributeEditor.setValue() which I missed when applying the patch. That caused the problem with updating of the priority editor. (In reply to comment #5) > I deliberately had put that mechanism in the subclasses that needed it rather > than in AbstractAttributeEditor because I was afraid of introducing bugs like > this. Maybe it would be better to undo your change and add it to whichever > subclass I apparently missed. For which attribute editor did you see the > problem? The reason for the follow up patch is described on bug 343005 comment 8. You make a good point about the intended behavior of setValue() though. My thinking was that setValue() would update the value and control but given that it is not consistently implemented that way the proposed solution of looping through the event handler mechanism seems preferable. I'll take another review pass through the attribute editors to make sure we consistently filter refresh events that are triggered by editing controls.
(In reply to comment #6) > The problem is that the original patch on bug 343005 changed the behavior of > PriortyAttributeEditor.setValue() which I missed when applying the patch. That > caused the problem with updating of the priority editor. Well, the change was just to remove the call to updateEditor which became unnecessary once it was auto-refreshing. PriortyAttributeEditor was working prior to the follow up patch which prevents that auto-refresh from occuring. > > (In reply to comment #5) > > I deliberately had put that mechanism in the subclasses that needed it rather > > than in AbstractAttributeEditor because I was afraid of introducing bugs like > > this. Maybe it would be better to undo your change and add it to whichever > > subclass I apparently missed. For which attribute editor did you see the > > problem? > > The reason for the follow up patch is described on bug 343005 comment 8. You didn't specify which attribute editor you saw the problem with though. For example, TextAttributeEditor does not have the problem you described because if filters the events it triggers. > > You make a good point about the intended behavior of setValue() though. My > thinking was that setValue() would update the value and control but given that > it is not consistently implemented that way the proposed solution of looping > through the event handler mechanism seems preferable. I'll take another review > pass through the attribute editors to make sure we consistently filter refresh > events that are triggered by editing controls. Thanks!
(In reply to comment #7) > You didn't specify which attribute editor you saw the problem with though. For > example, TextAttributeEditor does not have the problem you described because if > filters the events it triggers. Sorry, I should have been more specific. I saw that with the New Comment field which is implemented by RichTextAttributeEditor.
Ah, I see the problem where undo works 1 character at a time for the new comment field as well as for text attribute editors, although I do not see the cursor jumping. Your change fixes the problem for both, but I don't understand why it fixes it for TextAttributeEditor, given that it should already be doing the same thing. So if you revert your change, I think it will break undo for TextAttributeEditor.
Created attachment 195923 [details] fix
I have committed the patch which sets a suppressRefresh flag when attributes are edited to ignore refresh events for most attribute editors. Sam, I don't think this was necessarily a problem in TextAttributeEditor but SWT behavior can differ between platforms when values on controls are set programmatically. Some platforms generate modification events in that case but other don't. Rob, Leo, Shawn, Sam, please watch for any oddities in regard to values not being correctly updated in the task editor as I have only tested the changes on Gtk so far. Thanks!
