Bug 439299 - [Import/Export] Expanded Export Wizard with sub-categories auto-collapses
Summary: [Import/Export] Expanded Export Wizard with sub-categories auto-collapses
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4   Edit
Hardware: All All
: P3 normal with 14 votes (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, helpwanted
Depends on:
Blocks:
 
Reported: 2014-07-09 20:11 EDT by Ian Trimble CLA
Modified: 2022-06-07 06:01 EDT (History)
13 users (show)

See Also:
daniel_megert: review? (mistria)


Attachments
Test plug-in project that contributes export categories and wizards (3.60 KB, application/x-zip-compressed)
2014-07-09 20:11 EDT, Ian Trimble CLA
no flags Details
Patch including equals() implementation for WizardCollectionElement class (2.44 KB, patch)
2016-05-20 15:17 EDT, Craig Otis CLA
no flags Details | Diff
Patch including equals() and hashCode() implementations for WizardCollectionElement class (2.80 KB, patch)
2016-05-20 15:28 EDT, Craig Otis CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Trimble CLA 2014-07-09 20:11:19 EDT
Created attachment 244950 [details]
Test plug-in project that contributes export categories and wizards

When export wizard categories have only sub-categories as immediate children (no wizards as immediate children), and when export wizard gallery is first opened, when the top-level category is expanded, it briefly expands and then immediately collapses. If the category is not the first one expanded, or if the user attempts to expand again, it now remains expanded.

Repro:
1. Import the attached plug-in project into a dev workspace and launch a debug workspace.
2. File > Export... > attempt to expand "Test Category".
3. Note that it expands briefly and then collapses.
Comment 1 John Dallaway CLA 2015-01-08 11:51:01 EST
I see a similar problem with the new project wizard in platform 4.4.1. Expanding a top-level category node using the very first mouse click after the project wizard has been invoked can result in strange vertical scroll bar behaviour or immediate collapse of the category node that has just been expanded. Clicking elsewhere in the wizard and then expanding the category works correctly.
Comment 2 Roberto A CLA 2015-05-25 08:54:11 EDT
I have the same problem with Import wizard in eclipse Luna (4.4.2) and Mars(4.5.0M7) over Windows 7 OS.
Comment 3 Adam Bialas CLA 2015-05-25 15:56:14 EDT
Same for me (+1 vote)
Comment 4 Craig Otis CLA 2016-05-20 15:17:01 EDT
Created attachment 261909 [details]
Patch including equals() implementation for WizardCollectionElement class

I've attached a proposed patch for this issue. In my debugging, it seems the TreeViewer of the ImportExportPage is refreshed when the filter text changes.

As this text/field is selected by default when the page is shown, clicking anywhere outside the view triggers a change event, causing the tree to refresh and re-evaluate the collapsed/expanded state of its children. However, this relies on the equality of its items. See line ~2769 of AbstractTreeViewer.class:

setExpanded(item, expanded.containsKey(newElement));

The newElement *is a* WizardCollectionElement. Despite 'expanded' (a CustomHashtable) containing a WizardCollectionElement with matching properties, reference equality is instead used, which fails.
Comment 5 Craig Otis CLA 2016-05-20 15:28:45 EDT
Created attachment 261910 [details]
Patch including equals() and hashCode() implementations for WizardCollectionElement class

New patch, improves existing by also implementing hashCode(). Sorry - meant to include this in the original.
Comment 6 Andrey Loskutov CLA 2016-05-20 15:54:43 EDT
(In reply to Craig Otis from comment #5)
> Created attachment 261910 [details]
> Patch including equals() and hashCode() implementations for
> WizardCollectionElement class
> 
> New patch, improves existing by also implementing hashCode(). Sorry - meant
> to include this in the original.

Craig, would you mind to create Gerrit patch? It is easier to track and review patches.
Comment 7 Craig Otis CLA 2016-05-21 14:45:47 EDT
Absolutely - just waiting on some CLA sign-off, then will push to Gerrit.
Comment 8 George C. Hetrick CLA 2017-04-03 10:02:16 EDT
Is this patch scheduled for a release?
Comment 9 Chris Lake CLA 2018-01-21 17:15:59 EST
Any updates on this issue, still apparent in Oxygen 1a.
Comment 10 Craig Otis CLA 2018-01-21 17:36:53 EST
Yea, I have this patched locally but I'm struggling to get this Gerrit system set up.

I have my Gerrit account, the SSH key is configured, I've checked out:

[remote "origin"]
	url = git://git.eclipse.org/gitroot/platform/eclipse.platform.ui.git
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master

And I'm sitting on 'master'. I create my commit, and try to push:

craig:~/projects/eclipse.platform.ui$ git push origin HEAD:refs/for/master
fatal: remote error: access denied or repository not exported: /gitroot/platform/eclipse.platform.ui.git

Not sure where to go from here. I'm happy to contribute, but I'm not familiar with this process.
Comment 11 Andrey Loskutov CLA 2018-01-21 17:41:30 EST
(In reply to Craig Otis from comment #10)
> Yea, I have this patched locally but I'm struggling to get this Gerrit
> system set up.
> 
> I have my Gerrit account, the SSH key is configured, I've checked out:
> 
> [remote "origin"]
> 	url = git://git.eclipse.org/gitroot/platform/eclipse.platform.ui.git
> 	fetch = +refs/heads/*:refs/remotes/origin/*
> [branch "master"]
> 	remote = origin
> 	merge = refs/heads/master
> 
> And I'm sitting on 'master'. I create my commit, and try to push:
> 
> craig:~/projects/eclipse.platform.ui$ git push origin HEAD:refs/for/master
> fatal: remote error: access denied or repository not exported:
> /gitroot/platform/eclipse.platform.ui.git
> 
> Not sure where to go from here. I'm happy to contribute, but I'm not
> familiar with this process.

You should use https://git.eclipse.org/r/platform/eclipse.platform.ui for push. Note the changed protocol and the gitroot -> r change. SSH is for committers only.
Comment 12 Craig Otis CLA 2018-01-21 17:45:49 EST
Thanks, I tried that, but I'm getting an auth failure:

craig@Machine:~/projects/eclipse.platform.ui$ git push origin HEAD:refs/for/master
Username for 'https://git.eclipse.org': craigotis@gmail.com
Password for 'https://craigotis@gmail.com@git.eclipse.org':
remote: Unauthorized
fatal: Authentication failed for 'https://git.eclipse.org/r/platform/eclipse.platform.ui.git/'

I even visited https://git.eclipse.org in my browser, logged out, and logged back in (successfully) with the same credentials I'm providing on the CLI.
Comment 13 Andrey Loskutov CLA 2018-01-21 17:48:23 EST
(In reply to Craig Otis from comment #12)
> Thanks, I tried that, but I'm getting an auth failure:
> 
> craig@Machine:~/projects/eclipse.platform.ui$ git push origin
> HEAD:refs/for/master
> Username for 'https://git.eclipse.org': craigotis@gmail.com
> Password for 'https://craigotis@gmail.com@git.eclipse.org':
> remote: Unauthorized
> fatal: Authentication failed for
> 'https://git.eclipse.org/r/platform/eclipse.platform.ui.git/'
> 
> I even visited https://git.eclipse.org in my browser, logged out, and logged
> back in (successfully) with the same credentials I'm providing on the CLI.

Your https *Gerrit* password differs from the eclipse.org account. Go to https://git.eclipse.org/r/#/settings/http-password and use the credentials shown there.
Comment 14 Eclipse Genie CLA 2018-01-21 17:51:19 EST
New Gerrit change created: https://git.eclipse.org/r/115771
Comment 15 Craig Otis CLA 2018-01-21 17:53:05 EST
Thanks for all the help Andrey. I was following 3-4 guides for this in parallel, and got a bit mixed up. All set now.
Comment 16 Andrey Loskutov CLA 2018-01-21 18:00:06 EST
I have no bandwidth for review & verify it in M5. Patch looks good, but we should verify if this doesn't brake anything else.

@Platform UI team: anyone to verify the patch?
Comment 17 Eclipse Genie CLA 2018-01-21 18:12:35 EST
New Gerrit change created: https://git.eclipse.org/r/115772
Comment 18 Eclipse Genie CLA 2018-02-03 09:16:53 EST
New Gerrit change created: https://git.eclipse.org/r/116661
Comment 19 Mickael Istria CLA 2018-05-14 06:15:06 EDT
Thanks Craig.
While the proposed patch seems good, I'm wondering whether it's optimal. As you have investigated the code, the question I'd like to ask is "why do we get new instances of WizardCollectionElement?". I would expect the instances to be created only once and reused and that the default object equality would work, but you patch seems to highlight that instances are recreated everytime. I don't think it's a desired behavior.
Comment 20 Craig Otis CLA 2018-08-30 12:16:19 EDT
Hi Michael,

Unfortunately my patch is 2.5 years old, and I've since switched to IntelliJ. I won't be able to improve the patch, so if someone else wants to investigate a more optimal solution, that's great - but this is the best I can offer.

Thanks
Comment 21 Mickael Istria CLA 2018-08-30 12:57:16 EDT
Hi Craig,
Too bad you moved away before your patch was considered. Do you have a colleague or someone you work with who would have interest on following up on that patch? If yes, could you please provide guidance to this person to come to the right channels and contribute to fix this.
In any case, as the patch is IMO not optimal and you're not likely to work on it, I'm marking it as abandoned.
Comment 22 Nicole Behlen CLA 2018-09-21 16:34:29 EDT
I debugged the problem too, and I came to the same conclusion as Craig did, and I think the fix is correct. 

Afaik the WizardCollectionElement are created newly, due to the fact that with an active filter or active activities, the wizards/childs filtered out, are not added to the wizards list of the new WizardCollectionElement.
I do not know which side effects it would have, if the filtered Wizards would be removed from the original WizardCollectionElement, but I guess it would require reload the whole categories again and again, everytime the filter or activities setting changes.

Is there any chance to apply Craigs patch?
Comment 23 Eclipse Genie CLA 2020-11-24 01:31:24 EST
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.
Comment 24 Marcin Junger CLA 2020-12-31 07:11:38 EST
This is still unresolved, even though the possible fix is in gerrit.

Please apply the fix to Eclipse.
Comment 25 Mickael Istria CLA 2021-01-04 09:40:01 EST
(In reply to Marcin Junger from comment #24)
> This is still unresolved, even though the possible fix is in gerrit.

All the changes I see are abandoned, which means that their author have not been willing to finalize them into a mergeable state.

> Please apply the fix to Eclipse.

Please submit a patch and we'll work in reviewing and hopefully merging it with high priority.

I'm reopening this issue as you expressed it's still relavant.
Comment 26 Lars Vogel CLA 2022-05-31 05:23:26 EDT
(In reply to Craig Otis from comment #20)
> Hi Michael,
> 
> Unfortunately my patch is 2.5 years old, and I've since switched to
> IntelliJ. I won't be able to improve the patch, so if someone else wants to
> investigate a more optimal solution, that's great - but this is the best I
> can offer.
> 
> Thanks

Thanks Craig for your work, your patch has been moved to https://github.com/eclipse-platform/eclipse.platform.ui/pull/89
Comment 27 Lars Vogel CLA 2022-06-07 06:01:42 EDT
Fixed via https://github.com/eclipse-platform/eclipse.platform.ui/pull/89

Thanks again, Craig.