| Summary: | EditorPart#isSaveOnCloseNeeded() not called when closing Editor Part | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | <h1055071> |
| Component: | UI | Assignee: | Oleg Besedin <ob1.eclipse> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, elias, Lars.Vogel, p.beauvoir, pwebster, remy.suen |
| Version: | 4.2 | ||
| Target Milestone: | 4.2 M7 | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
This works when using Ctrl+W but not when clicking on the 'X'. When the 'X' is clicked on, we first ask the user to save the part before we close it as the isSaveOnCloseNeeded() concept is not in Eclipse 4. (In reply to comment #1) > This works when using Ctrl+W but not when clicking on the 'X'. > > When the 'X' is clicked on, we first ask the user to save the part before we > close it as the isSaveOnCloseNeeded() concept is not in Eclipse 4. So, can it be fixed? Or is this intended for Ecliopse 4? (In reply to comment #2) > So, can it be fixed? It needs to be fixed for compatibility reasons anyway, yes. Will / has this ever shown up in the IDE itself ? If not then we'll likely defer this to 4.2.1... So the "compatability" layer will be broken for Eclipse 4.2? (This and other bugs I've found). (In reply to comment #5) > So the "compatability" layer will be broken for Eclipse 4.2? (This and other > bugs I've found). no, there will be bugs in it, same as 3.8 (which has bugs numbering in the thousands) PW (In reply to comment #4) > Will / has this ever shown up in the IDE itself ? If not then we'll likely > defer this to 4.2.1... Maybe not, but it shows up in our RCP application. We won't be "upgrading" to Eclipse 4 anytime soon. (In reply to comment #4) > Will / has this ever shown up in the IDE itself ? If not then we'll likely > defer this to 4.2.1... In the SDK there are several callers (outside Platform UI) and one override. (In reply to comment #8) > In the SDK there are several callers (outside Platform UI) and one override. I would be concerned about the overrides, not really worried about the callers. (In reply to comment #9) > (In reply to comment #8) > > In the SDK there are several callers (outside Platform UI) and one override. > > I would be concerned about the overrides, not really worried about the callers. Agreed. We also have to check whether only EditorPart.isSaveOnCloseNeeded() is missing, or whether this is a general problem i.e. affects org.eclipse.ui.ISaveablePart.isSaveOnCloseNeeded(). There, we have several overrides. My use of this is that our RCP tool (http://archi.cetis.ac.uk) uses several GEF-based Editor parts to edit different diagrams for the same model. The "dirty" state is maintained for the model, not for each Editor part. Thus we don't want to prompt the user to save the editor part when it is closed. This was easy to implement in Eclipse 3.x by simply returning false when over-riding sSaveOnCloseNeeded(). Now in the e4 compatibility layer this no longer works so our app cannot be used beyond 3.x. (In reply to comment #13) > Fixed: > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8ab339d1d4c6bc5ba32c44bdf870b29bbe1903fa This feels off to me. This means a request to save will simply be ignored if the editor returns 'false' with isSaveOnCloseNeeded(), no? The problem here is that we have no way of distinguishing between a save request and a save request that's been triggered by a close request. (In reply to comment #14) > (In reply to comment #13) > > Fixed: > > > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8ab339d1d4c6bc5ba32c44bdf870b29bbe1903fa > > This feels off to me. This means a request to save will simply be ignored if > the editor returns 'false' with isSaveOnCloseNeeded(), no? > > The problem here is that we have no way of distinguishing between a save > request and a save request that's been triggered by a close request. The ISaveHandler#promptToSave() is not called on a normal save action (Ctrl+S/Menu/Toolbar). It is called on part close. As far as I know, normal save is not affected by this change. (In reply to comment #15) > The ISaveHandler#promptToSave() is not called on a normal save action > (Ctrl+S/Menu/Toolbar). It is called on part close. As far as I know, normal > save is not affected by this change. If someone goes through the EPartService's savePart(MPart, boolean) method then this becomes a problem. (In reply to comment #16) > (In reply to comment #15) > > The ISaveHandler#promptToSave() is not called on a normal save action > > (Ctrl+S/Menu/Toolbar). It is called on part close. As far as I know, normal > > save is not affected by this change. > > If someone goes through the EPartService's savePart(MPart, boolean) method then > this becomes a problem. Not as much a problem as breaking backwards compatibility? (In reply to comment #17) > (In reply to comment #16) > > If someone goes through the EPartService's savePart(MPart, boolean) method then > > this becomes a problem. > > Not as much a problem as breaking backwards compatibility? Not my decision to make. Though having new API not do something correctly with pre-existing clients could also be interpreted as breaking backwards compatibility. In any case, if there are no plans to reopen this and fix it correctly, then please open a new bug to track the problem with the now broken 4.x API. (In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > If someone goes through the EPartService's savePart(MPart, boolean) method then > > > this becomes a problem. > > > > Not as much a problem as breaking backwards compatibility? > > Not my decision to make. Though having new API not do something correctly with > pre-existing clients could also be interpreted as breaking backwards > compatibility. > > In any case, if there are no plans to reopen this and fix it correctly, then > please open a new bug to track the problem with the now broken 4.x API. And the 4.x API isn't already broken? As well as the totally unusable broken "compatibility layer"? With this fiasco I am abandoning Eclipse RCP development. With e4, Eclipse jumped the shark... (In reply to comment #18) > In any case, if there are no plans to reopen this and fix it correctly, then > please open a new bug to track the problem with the now broken 4.x API. Done: bug 377519 . It is not possible to fix it "correctly" at this point of the release cycle. A sizable block of functionality needs to be moved from 3.x into e4 code. Verified in I20120430-1800. This is a bug again in E4 RC4. Build id: I20130605-2000 Please re-open. isSaveOnCloseNeeded() is not being called just as before. Back to Eclipse 3.8 I go...again... :sigh: Re-submitted as Bug #411465 |