Bug 445021 - [Navigator] Bugged forward / back navigation
Summary: [Navigator] Bugged forward / back navigation
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 4.4   Edit
Hardware: All Windows 8
: P3 major with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-24 17:05 EDT by James Hutchison CLA
Modified: 2020-06-04 11:17 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description James Hutchison CLA 2014-09-24 17:05:39 EDT
Forward/back navigation is broken. I don't understand why it has been for so long but I've seen this in previous versions of eclipse (both PC and mac) and I'm a little frustrated such a fundamental piece of functionality which is nothing more than a stack has been broken for so long. I've finally taken the time to post this bug report and its very easy to reproduce.

1. Start eclipse with an open project
2. In the project explorer open file A
3. W/ explorer, open file B
4. W/ explorer, open file C
5. W/ explorer, open file B again
6. Press back, note that nothing seems to have happened
7. Press back again, you are at file A
8. Press forward, you are file B
9. Press forward, you are at file C
10. Press backward, you are at file B
11. Press backward, you are at file A
12. W/ explorer, open file B
13. Note that you can no longer navigate backwards nor forwards

There may be other issues but I think this series of steps speaks for itself. There's also a very common situation where a file completely disappears from the navigation history which is probably related to this bug. That bug typically happens if you navigate after pressing back without pressing forward.

Also, while you're at it, I think navigation should only occur between *open* files.

Thanks!
Comment 1 Daniel Rolka CLA 2014-09-25 04:23:07 EDT
The bug is out of the scope of Mars so nobody works on it. The patch would be appreciated - http://wiki.eclipse.org/Platform_UI/How_to_Contribute 

thanks a lot for your help,
Daniel
Comment 2 John Hendrikx CLA 2017-10-04 16:56:21 EDT
I found something odd, but am still unable to reproduce this with an empty workspace.

On big workspaces (several dozens projects open) this happens consistently for me.  And while attempting to isolate the problem I did notice that you can "avoid" this bug by making sure the Package Explorer has focus *before* doing 
 a double click (so, first focus it, then double click on a file).  I'll spell it out:

Let's assume you have the files A, B, C (all Java files):

1) Click on the navigation down arrow to see what file is the "previous" file.  This removes the focus from Package Explorer (which we want).

2) Now double click "A" in package explorer (make sure it is a double click, not a single click, followed by a double click).  Your previous file may or may not be in the history.

3) Now double click "B" in package explorer.  Check the navigation history again with the navigation arrow.  Notice that "A" is not in the history (yet!).

4) Now double click "C" in package explorer.  Check the navigation history again with the navigation arrow.  Notice that "A" now is in the navigation history(!!).

5) You can repeat this as often as you like, and every time it adds the second to last file to the history but not the current one until the next step...

If you do the above, but first give Package Explorer the focus each time, so first single click package explorer, and then double click a file then the history works as expected (same if you first select a file, then use "F3" to navigate to it).

That's gotta be a wierd bug...
Comment 3 John Hendrikx CLA 2017-10-10 09:59:45 EDT
The problem is almost certainly in the piece of code below in the NavigationHistory class.

Let's say you start with a history that looks like:

>>Index: 0 A (with A being the active editor)

When the bug occurs, MarkEditor is called several times in a row, for some reason it is first called with the *current* open editor "A" (when the bug happens), rapidly followed by several calls of the newly opened editor "B".

Because of the "workaround" in this code to ignore extra entries while we wait for some display code to async update the history (WTF!!!), only "A" is added to the history, while "B" is ignored.

The history is unchanged as a result (or perhaps a merge occured with "A").

If the bug occurs again after that (you double click on "C"), MarkEditor is called with "B" first, and is happily added to the history (even though it should be there already).  This is followed by several MarkEditor calls for "C", but those are ignored again.  The history (with editor "C" being active) now is:

Index: 0 A
>>Index: 1 B

Even though it should be (as I just opened "C"):

Index: 0 A
Index: 1 B
>>Index: 2 C

Now the code below looks bad to me.  I cannot even begin to understand why a low-level piece of code that maintains a Navigation History would need to do anything on the SWT thread, but there it is:


    public void markEditor(final IEditorPart part) {
        if (ignoreEntries > 0 || part == null) {
			return;
		}
        /* Ignore all entries until the async exec runs. Workaround to avoid
         * extra entry when using Open Declaration (F3) that opens another editor. */
        ignoreEntries++;
        getDisplay().asyncExec(() -> {
		    if (--ignoreEntries == 0) {
		        if (part.getEditorSite() instanceof EditorSite) {
					EditorSite site = (EditorSite) part.getEditorSite();
					Control c = (Control) site.getModel().getWidget();
		            if (c == null || c.isDisposed()) {
						return;
					}
		            NavigationHistoryEntry e = getEntry(activeEntry);
		            if (e != null
		                    && part.getEditorInput() != e.editorInfo.editorInput) {
						updateEntry(e);
					}
		            addEntry(part);
		        }
		    }
		});
    }
Comment 4 John Hendrikx CLA 2017-10-10 10:09:54 EDT
Here a piece of the log I created while debugging this.

The current open editor is "DependencyException.java".  The history looked like this:

Index: 0 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Binder.java)> Details<Selection<offset: 0, length: 0>>
Index: 1 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Binding.java)> Details<Selection<offset: 0, length: 0>>
Index: 2 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/BindingException.java)> Details<Selection<offset: 0, length: 0>>
>>Index: 3 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/ClassInjectable.java)> Details<Selection<offset: 0, length: 0>>

As you can see, entry 3 is active, but it is not the same as the current open editor ("DependencyException.java").  It is infact pointing to the previous editor.

I now double click on "Injector.java", and got the following output:

MarkEditor: adding: org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/DependencyException.java)
MarkEditor: ignoring: org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Injector.java)
MarkEditor: ignoring: org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Injector.java)
MarkEditor: ignoring: org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Injector.java)
MarkEditor (async)
AddEntry: NavigationHistoryEntry = Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/DependencyException.java)> Details<Selection<offset: 429, length: 0>>
+++++ Added Entry +++++
Index: 0 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Binder.java)> Details<Selection<offset: 0, length: 0>>
Index: 1 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/Binding.java)> Details<Selection<offset: 0, length: 0>>
Index: 2 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/BindingException.java)> Details<Selection<offset: 0, length: 0>>
Index: 3 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/ClassInjectable.java)> Details<Selection<offset: 0, length: 0>>
>>Index: 4 Input<org.eclipse.ui.part.FileEditorInput(/ddif-core/src/main/java/hs/ddif/core/DependencyException.java)> Details<Selection<offset: 429, length: 0>>

Note that it adds the missing "DependencyException.java" editor first, and that gets added succesfully by the async call.  However, while we were waiting for the async call to complete, it tried to add "Injector.java", but can't because it gets ignored... the end result is that the history is still wrong and somehow seems to be lagging one step behind (see the Index list above, where Index 4 is now the previous open editor, but there is no index 5 with the current open editor...)