Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the issue when Window menu items are not removed upon closing images #30

Merged
merged 4 commits into from Jul 23, 2014

Conversation

dscho
Copy link
Contributor

@dscho dscho commented Jul 23, 2014

The problem is actually that ImageJ 1.x gets confused when adding new items, putting them into the wrong spot (when plugins were installed into the Window menu). As a consequence wrong menu items are removed when closing images.

In addition to the fix, the changes also include a new API to allow for conditional patching, contingent on ImageJ 1.x version ranges. The reason for this change is that this developer anticipates ImageJ 1.x to be changed in a way that breaks ij1-patcher. When that happens, we can make use of the new API to unbreak it quickly.

This pull request addresses and supersedes imagej/imagej-legacy#68.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Due to the scheme that sees versions like 1.49c13 before 1.49c, let
alone 1.49 final, a simple String comparison is unfortunately not good
enough.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The ij1-patcher has to work with a wide range of ImageJ 1.x versions.
The reason is simple: every once in a while, previously working plugins
stop working because ImageJ 1.x development is not *quite* as
backwards-compatible as it could be. In such cases, users are basically
stuck with older versions, leading to the famous 'ImageJ icon gallery'
on the desktop.

Sometimes ImageJ 1.x changes crucially so that completely different
fixes are needed for different versions. To that end, we introduce a
function that lets you verify that the current ImageJ 1.x version is at
least the specified one, for conditional patching.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When ImageJ 1.x adds images to the Window menu, it does not care whether
there were already some plugin installed or not; it always appends the
new menu item. Instead, it should insert the correct position.

This fixes imagej/imagej-legacy#68.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Contributor Author

dscho commented Jul 23, 2014

@ctrueden I am not quite sure whether it is appropriate to call java.awt.Menu#insert(java.awt.MenuItem, int) with an index at the end of the menu (effectively appending an item). I tested it and it works on MacOSX' Java 6, also, logic should dictate that add would be implemented simply as an insert with the index set to the number of menu items already in the menu, but I am a little worried because I cannot even find specifics in the Java 7 API documentation whether a position exceeding the current positions is considered valid... Any insights?

@ctrueden
Copy link
Member

I am not quite sure whether it is appropriate to call java.awt.Menu#insert(java.awt.MenuItem, int) with an index at the end of the menu

I think that is completely appropriate, and the intended way to append to an AWT menu.

This pull request addresses and supersedes imagej/imagej-legacy#68.

As an aside, in case you didn't know, you can convert a vanilla issue into a pull request using the GitHub API. 😁

Anyway, in my tests, this patch fixes the Window menu problems. However, it does not address another bug: closing and reopening the same image results in -1 being appended the second time (and -2 the third time, etc.). This suggests that somehow the images are not being completely closed. This might be a consequence of imagej/imagej-legacy#67; for now I'll leave a comment there about this issue. If the problem still occurs after that issue is fixed, we can file another and continue investigating.

In any case, regarding this PR: LGTM! 👍

ctrueden added a commit that referenced this pull request Jul 23, 2014
Fix the issue when Window menu items are not removed upon closing images
@ctrueden ctrueden merged commit 20360a6 into master Jul 23, 2014
@ctrueden ctrueden deleted the window-menu branch July 23, 2014 15:54
@dscho
Copy link
Contributor Author

dscho commented Jul 23, 2014

Thanks for the tip with converting issues into pull requests (although that would not apply here because I originally thought it was an imagej-legacy issue, but then turned it into an ij1-patcher PR)!

And thanks for the reminder regarding the suffixes. I will investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants