Bug 502038 - [CommonNavigator] Filters are not activated correctly in NavigatorFilterService
Summary: [CommonNavigator] Filters are not activated correctly in NavigatorFilterService
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.7 M3   Edit
Assignee: Jan Rosczak CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday
Depends on:
Blocks: 167414
  Show dependency tree
 
Reported: 2016-09-23 02:30 EDT by Jan Rosczak CLA
Modified: 2016-09-26 12:55 EDT (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Rosczak CLA 2016-09-23 02:30:59 EDT 
The condition when a filter is activated and when a filter is deactivated needs to be the other way around. Currently a filter is deactivated when the boolean variable "toMakeActive" is true. This is clearly wrong.
Comment 1 Lars Vogel CLA 2016-09-23 03:00:05 EDT 
Patches are welcome
Comment 2 Patrik Suzzi CLA 2016-09-23 13:16:02 EDT 
Probably is to be fixed in NavigatorFilterService#setActive(..), to match the usage we see in ToggleFilterAction#run()
Comment 3 Jan Rosczak CLA 2016-09-24 02:16:53 EDT 
I have a patch pending and will submit it soon
Comment 4 Mickael Istria CLA 2016-09-24 12:57:38 EDT 
Thanks Jan. This indeed seems to be a bug. Do you have any example of workflow that highlight this bug in the user perspective?
Comment 5 Jan Rosczak CLA 2016-09-26 02:36:12 EDT 
It seems as if the class ToggleFilterAction which is the only class which calls the method setActive(String aFilterId, boolean toMakeActive) on the NavigationFilterService is not used anywhere.

Therefore I think that currently no workflow exists to highlight this bug. Only when I implement the MRU filters feature I make use of the ToggleFilterAction and the bug surfaces.
Comment 6 Eclipse Genie CLA 2016-09-26 02:42:00 EDT 
New Gerrit change created: https://git.eclipse.org/r/81880
Comment 8 Mickael Istria CLA 2016-09-26 12:55:23 EDT 
Patch merged, thanks Jan!