| Summary: | [misc] menu actions disabled after rename and then dirtying editor | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Tom Hofmann <eclipse> | ||||
| Component: | Text | Assignee: | Dani Megert <daniel_megert> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P2 | CC: | daniel_megert, david_williams, dirk_baeumer | ||||
| Version: | 3.1 | ||||||
| Target Milestone: | 3.1 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux-GTK | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Tom Hofmann
Cannot reproduce this one. I see Undo Typing in both the Edit menu and the popup. Is it possible you changed the active part somewhere along the way? Some parts don't show undo/redo (they are disabled) so that could explain the behavior. investigate RC1 - I don't think this related to bug #93955 since the problem in this bug is a mismatched pulldown/popup menu. Reproduced on M7 final build. Note that once you are in this state, undo remains disabled in B.java until you activate a different editor. Then switch back to the original editor and the undo pulldown is updated. The behavior is very similar to that of bug #92070. Tom, can you reproduce this by simply "dirtying" the editor as written in the summary or is it actually the rename that triggers the bug? (In reply to comment #5) > Tom, can you reproduce this by simply "dirtying" the editor as written in the > summary or is it actually the rename that triggers the bug? The rename is required. After simply creating a new class and dirtying it, the menu is correctly enabled. Just reproduced on N20050516 with the steps from comment 0. Note: the menu is permanently wrong for this editor after this happens. Changing focus and coming back, switching editors and back etc. all don't enable the menu. oops, not true: switching editors does enable it, but not switching view. adapting summary I think I've tracked this down. As part of fixing bug #89393, the undo/redo action handlers are recreated every time the editor input changes. (See bug #89393 comment #8). This has always bothered me a little, because it leaves multiple actions listening to part events and operation history events and updating the action text, but since it did not cause any leaks I never really looked into what happens when multiple actions are created by a single part and registered for the same action definition. In examining the RetargetAction code, it would seem possible that any of the created action handlers could update the action text and enablement for the site's action bars. This means that depending on the order of operation history event listeners (and also the property change listeners, etc.), any of the previously created actions could update the text and enablement last, causing the wrong enablement and/or text (since the old actions are associated with an old undo context.) I added code in AbstractTextEditor to dispose of any old undo/redo actions created before creating new ones, so that the "dead" handlers no longer update text and enablement as part of listening to operation history events. This solves the problem. Attaching a patch to AbstractTextEditor and moving to platform text. Created attachment 21310 [details]
AbstractTextEditor patch
The patch does something good, it disposes the old undo/redo actions but it does not fix the problem. The cause for this bug was that while we re-create the undo/redo action handlers we forgot to re-register them with the global action handler. I've fixed this up. The fix is ready but I will commit only after having verified all the previously broken scenarios (thanks Susan sending me the list). This will be done tomorrow. I also plan to investigate whether there's a way around re-creating the undo manager upon editor input change. Verified and committed to HEAD. For the undo manager reuse investigation item see bug 96756. start verifying Verified in 3.1 RC1. |