Bugzilla – Full Text Bug Listing
| Summary: | [Undo] [PropertiesView] [Outline] Undo/redo action handlers can't be reused inside page book view pages | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Nick Edgar <n.a.edgar> |
| Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P5 | CC: | douglas.pollock |
| Version: | 3.2 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows 2000 | ||
| Whiteboard: | stalebug | ||
Just to clarify further how the EMF case works:
The subclass of ExtendedPropertySheetPage is a local class inside the generated editor. Its getActionBarContributor() returns the editor's action bar contributor as an EditingDomainActionBarContributor. Its shareGlobalActions method is:
public void shareGlobalActions(IPage page, IActionBars actionBars)
{
if (!(page instanceof IPropertySheetPage))
{
actionBars.setGlobalActionHandler(ActionFactory.DELETE.getId(), deleteAction);
actionBars.setGlobalActionHandler(ActionFactory.CUT.getId(), cutAction);
actionBars.setGlobalActionHandler(ActionFactory.COPY.getId(), copyAction);
actionBars.setGlobalActionHandler(ActionFactory.PASTE.getId(), pasteAction);
}
actionBars.setGlobalActionHandler(ActionFactory.UNDO.getId(), undoAction);
actionBars.setGlobalActionHandler(ActionFactory.REDO.getId(), redoAction);
}
So it's directly setting the editor's undoAction and redoAction as the property sheet page's undo and redo actions.
(Aside: the if statement is there because the same approach is used for the editor's ContentOutlinePage. In the outline, it makes sense to delegate the selection-based actions as well as undo/redo.)
In looking at the use of IWorkbenchPartSite inside the action handlers, it seems that for most cases (obtaining the shell, workbench window, etc.) we can get by with using IWorkbenchSite instead. There is only one place where we rely on being tied to a part. Currently, we associate the life cycle of the action handler with that of the part. So we have to know when "our" part is closed in order to dispose the action handler. We would need another approach for disposing the action handler to get around this, such as pushing the responsibility back on the client. This is leak-prone. See discussion in bug #88352 c#11 and beyond. One possibility is to do the disposal when we can get a part (check for instanceof IWorkbenchPartSite) and otherwise make the client do it. But this seems confusing/leakprone. Thoughts? I don't have any extra thoughts off-hand. Maybe Doug does given his recent work on localized services. My world view is much different. If you were registering handlers through PageSite, then this wouldn't be an issue.
PageSite.dispose -> IServiceLocator.dispose -> IHandlerService.dispose -> IHandler.dispose.
As it stands right now (HEAD), all you would need to do to get the key binding working is the following:
IHandlerService service =
(IHandlerService) pageSite.getService(IHandlerService.class);
service.activateHandler("org.eclipse.ui.edit.undo", undoHandler);
Making OperationHistoryActionHandler implement IHandler would make this feasible. If no site is provided, simply don't hook the listener. Of course, until the menus part is finished, the top-level undo menu item still wouldn't work properly.
Crazy thought, but maybe RetargetAction should look for a new-style handler from the active part if it doesn't provide an IAction handler. I had that exact same thought as I wrote comment #4. I think it's a good idea, but I don't have the time to do it. If either you or Susan had time to write a patch for it, I could look at it (and provide guidance). I'm a bit overwhelmed with other stuff at the moment, and suspect Nick would be much faster than me anyway. Nick? I can't for M5, but I have it on my list to look at after. Unfortunately, I won't be able to try that experiment for 3.2. changing prio and status to conform to plat-ui bug guidelines This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |
build I20060118 The constructor for OperationHistoryActionHandler and its subclasses take an IWorkbenchPartSite. For pages within a page book view (e.g. Outline, Properties), they don't have an IWorkbenchPartSite (though they do have an IPartSite, which extends IWorkbenchSite), so these actions can't be reused within a property sheet page or outline page. The workaround is for the page to delegate back to the source part's actions, or use them directly. EMF does this, for example. See EditingDomainActionBarContributor.shareGlobalActions(IPage, IActionBars), which is called from a generated subclass of ExtendedPropertySheetPage, which overrides setActionBars as follows: public void setActionBars(IActionBars actionBars) { super.setActionBars(actionBars); getActionBarContributor().shareGlobalActions(this, actionBars); } See also the "Undo in properties view" thread in the eclipse.platform newsgroup.