Bugzilla – Full Text Bug Listing
| Summary: | [CommonNavigator] Infinite loop in WorkingSetActionProvider.setWorkingSetFilter if misconfigured | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Fabio Zadrozny <fabiofz> | ||||||
| Component: | UI | Assignee: | Francis Upton IV <francisu> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | critical | ||||||||
| Priority: | P3 | CC: | bokowski, francisu, pwebster, remy.suen, shawn.minto, susan | ||||||
| Version: | 3.6 | Flags: | bokowski:
review+
remy.suen: review+ pwebster: review+ |
||||||
| Target Milestone: | 3.6 RC3 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Fabio Zadrozny
I will get a patch together this evening. Just as a note, if I add the resource filter context extension:
<extension point="org.eclipse.ui.navigator.viewer">
<viewerContentBinding viewerId="org.python.pydev.navigator.view">
<includes>
<contentExtension pattern="org.eclipse.ui.navigator.resources.filters.*"/>
</includes>
</viewerContentBinding>
</extension>
it does work. So, it seems something changed there -- the working set actions
used to work properly without adding it in 3.5 -- is this required now?
Anyways, it definitely shouldn't halt if it's not added (and it probably
shouldn't be required as those actions appear without setting it, but if that's
not the case, probably the working-set related actions shouldn't appear).
(In reply to comment #2) > Just as a note, if I add the resource filter context extension: > > <extension point="org.eclipse.ui.navigator.viewer"> > <viewerContentBinding viewerId="org.python.pydev.navigator.view"> > <includes> > <contentExtension pattern="org.eclipse.ui.navigator.resources.filters.*"/> > </includes> > </viewerContentBinding> > </extension> > > > it does work. So, it seems something changed there -- the working set actions > used to work properly without adding it in 3.5 -- is this required now? Yes, something did change here. In 3.5 the working set filters were directly on the CommonViewer so that when you changed a filter on the ProjectExplorer it would mess up the working set filter. In 3.6 the working set filters use the CNF filter mechanism, so the working set code for the ProjectExplorer depends on having the appropriate configuration for the working set filter. So if you are reusing this, then you need to do what you do above. > > Anyways, it definitely shouldn't halt if it's not added (and it probably > shouldn't be required as those actions appear without setting it, but if that's > not the case, probably the working-set related actions shouldn't appear). I agree that we should fix it so that if you make this configuration mistake it does not infinitely loop. Created attachment 169918 [details]
Proposed patch for RC3
This is only a problem if you are using the ProjectExplorer's working set actions and don't configure it properly by adding the (new) workingSetFilter. Though given all of this stuff is undocumented since it's not API there is no indication that you have to add this workingSetFilter if you are using the working set actions (I will probably need to update the compatibility notes for this case).
In any case, the failure to update your plugin.xml with the required filter earns a rather harsh sanction of an infinite loop.
This patch gives a more helpful message instead.
The problem is very easy to make happen, simply comment out the org.eclipse.ui.navigator.resources.filters.workingSet filter in the org.eclipse.ui.navigator.resources plugin.
Susan, Remy, Boris, Paul, can you guys have a look at this (of course I only need Boris plus 2 others). Why are you using an int "depth" instead of a boolean? Created attachment 169928 [details] Another alternative (In reply to comment #6) > Why are you using an int "depth" instead of a boolean? I think it's a bit more simple and expresses the recursion, so personally I like it. I find the boolean version harder to follow. But it's attached. Up to you. I'm fine with either version. PW +1 for the version with a boolean. The "int" made me wonder if you'd ever have a case with deeper recursion but I couldn't come up with a reason for needing it. I think even cleaner would be a solution with no recursion at all, by refactoring the loop searching for a workingSetFilter into its own method (and then calling that method twice from setWorkingSetFilter if needed). But let's fix this as suggested for now, I don't want to hold us up more than I already have. Think I like the integer version a little more myself but anyway... All tests are green on my box, +1. Released to HEAD 3.6RC3 *** Bug 314963 has been marked as a duplicate of this bug. *** |