Bugzilla – Full Text Bug Listing
| Summary: | File->Close menu item is disabled even when current editor is activated | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Keith Chong <keith.chong.ca> | ||||
| Component: | UI | Assignee: | Paul Elder <pelder.eclipse> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | bsd, daniel_megert, euthanasia_waltz, jpitman, pelder.eclipse, pwebster, xucxuc, yiming.sun | ||||
| Version: | 4.2.1 | Keywords: | helpwanted | ||||
| Target Milestone: | 4.3 M7 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 407085 | ||||||
| Attachments: |
|
||||||
|
Description
Keith Chong
I'm unable to reproduce on MacOS X and Linux using the latest milestone build towards 4.2.2. 1. I created a Java project and then imported 10 different XML files. 2. I opened each file using Open With > Default Editor. The Plug-in Spy (Shift-Alt-F1) reports the editor is a XMLMultiPageEditorPart from org.eclipse.wst.xml.ui 1.1.301.v201208170345 (editor id: org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorPart) 3. File > Close remains enabled. Could you try the Plug-in Spy and report back what editor type these files are opening with? I'm seeing this problem on both Windows and Mac OS X for other kinds of files as well. I've tried HTML and XML files with WTP installed, and Java and plain text files in an unmodified Eclipse SDK install, and they all have this problem. I used the M20121128-1200 4.2.2 build. 1. Created a Java project 2. Created a Java class. It automatically opened in the Java Editor. File > Close was enabled 3. Closed the Java file 4. Double-clicked to open the Java file. File > Close is now disabled. I can reproduce this on linux in 4.3 as well. PW *** Bug 397787 has been marked as a duplicate of this bug. *** The org.eclipse.ui.file.close handler has the following enabledWhen expression:
<with
variable="activeEditor">
<instanceof
value="org.eclipse.ui.IEditorPart">
</instanceof>
</with>
I suspect the issue arises from an ordering issue with how the contexts propagate changes to the active variables. Changing the active editor changes causing WorkbenchPage.updateActiveEditorSources() to update the window context's activeEditor. In tracing through the execution, the EvaluationService evaluates the enableWhen in the Workbench's context while the corresponding HandlerProxy#setEnabled() evaluates the same expression relative to the active-part's context *before* the active variant of the activeEditor variable has been been propagated to the active-part's context. So the expression evaluates to false for HandlerProxy and thus the menu item is disabled.
I am seeing the same as comment 5. I've gone a little further and taken a look at the EvaluationService, and how it retrieves 'activeEditor' from the context. Ultimately, it calls EclipseContext.getActive("activeEditor"). I did a couple of investigations of getActive(): 1) The Javadoc says it is the equivalent of getActiveLeaf().get(name), so I created a debug version of getActive() that compares that getActiveLeaf().get() with the value calculated by getActive(). 2) I instrumented the two calculations, to compare execution times, and to record any inconsistencies between the two calculation methods. A typical result: getActive() calls: 15680, 22ms (optimized), 15ms (unoptimized) getActive() inconsistencies: 782, names: [activeWorkbenchWindow.activePerspective, org.eclipse.ui.selection, activeEditor, activePart, activeWorkbenchWindow, activeContexts, selection, activeEditorId] Several observations on the results: 1) There are a significant number of times when the two methods of calculation do not yield the same result, including some calls looking for 'activeEditor'. 2) If my debug version uses the getActiveLeaf.get() result (instead of its calculated result), then the Close menu enables correctly. 3) There is no apparent performance advantage to the current getActive() implementation compared to getActiveLeaf().get(). In fact, the latter may be slightly faster. (The execution times are the total for all calls and are measured via System.nanoTime(), but converted to milliseconds so I can actually read them). Paul, can you reproduce with the patch from Bug 394171? attachment 225996 [details] PW Paul W: No, the patch does not fix this issue. Created attachment 226842 [details]
Proposed fix
The attached patch simplifies EclipseContext.getActive() to do what it's Javadoc comment says: getActiveLeaf().get(). Profiling indicates that there is no significant performance difference between the current implementation and the patch - in fact, the patch is sometimes faster!
(In reply to comment #9) > Created attachment 226842 [details] > Proposed fix But it's no longer a tracked value ... so when the active child changes (say switching parts) or the value in the active child changes, any RATs are not re-run PW I've pushed the change up to Gerrit for us to review there: https://git.eclipse.org/r/10749 I'll also note, I get a failure in org.eclipse.e4.core.tests.CoreTestSuite from org.eclipse.e4.core.tests. PW *** Bug 400960 has been marked as a duplicate of this bug. *** I've pushed a suggested fix to https://git.eclipse.org/r/#/c/11912/ It's a new changeset because it is in a different repo than the original fix. I could reproduce the problem fairly easily and then with this patch I haven't been able to reproduce it since (it still needs some more testing). Paul, could you test this out in some of your scenarios, see that it still works? PW I've released this so it can get some broader testing, but I think it's still worth it us doing some targetted testing if you could. PW Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f1cd3bf45ceb9e023bacbf2577eb28bea52f0260 PW . Paul, this still does not work for me. 1. download http://download.eclipse.org/eclipse/downloads/drops4/N20130418-0900/ 2. install it 3. start with new workspace 4. close Welcome 5. paste this into the 'Package Explorer': class Bug {} ==> File > Close and File > Close All are disabled! (In reply to comment #17) > > 1. download > http://download.eclipse.org/eclipse/downloads/drops4/N20130418-0900/ > 2. install it > 3. start with new workspace > 4. close Welcome > 5. paste this into the 'Package Explorer': I can reproduce. Closing an editor seems to enable it, and it works from then on. But, closing the last editor doesn't disable it. PW Close All is bug 400960 PW Seems to be interaction with my fix for bug 382839 Fix: https://git.eclipse.org/r/#/c/12098/ PW Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=b976b22fca1d2c3b279d920137e2fcd1beccc927 PW (In reply to comment #21) > Released as > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=b976b22fca1d2c3b279d920137e2fcd1beccc927 > > PW Verified that it fixes the scenario from comment 0 and comment 17. Paul, it also fixes the enabling of 'Close All' for me. *** Bug 392149 has been marked as a duplicate of this bug. *** |