Bugzilla – Full Text Bug Listing
| Summary: | [Markers] Marker views redirect "Preferences" command | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Philipp Kursawe <phil.kursawe> | ||||
| Component: | UI | Assignee: | Prakash Rangaraj <prakash> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | bokowski, daniel_megert, deepakazad, markus.kell.r, prakash, pwebster, remy.suen, tomasz.zarna | ||||
| Version: | 3.6 | ||||||
| Target Milestone: | 3.7 M5 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | 231081, 270007 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Philipp Kursawe
Yes, it supplies its own MarkerPreferencesDialog. That's deliberate. PW I understand that. But is it supposed to hook into the Preferences command? I have a menu contribution on my toolbar button that is supposed to show *my* preference page with the preference command and preferencePageId set to my preference page. All it does, when the Problems view is active, is to show the problem views preference page. That's clearly not supposed to happen. I see that it indeed provides a handler for "org.eclipse.ui.window.preferences" which is wrong, imho. It should provide a preferences page or create its own command and handler. (In reply to comment #3) > I see that it indeed provides a handler for "org.eclipse.ui.window.preferences" > which is wrong, imho. It should provide a preferences page or create its own > command and handler. But the Markers view wants File>Preferences to open its preferences dialog. Providing its own handler does exactly what it wants. PW But it overwrites not "file->preferences" (where is that anyway?). It overwrites the default handler for "window->preferences". I have a toolbar dropdown button on the main-toolbar that uses the "window-preferences" command to display its preferences. As soon as the "Problems View" has the focus my dropdown button menu entry will display the problems view preference dialog. That is not expected. The problems view should provide its own command and leave the "window-preferences" handler alone. Or it could just provide its own preference page and hook up with the preference pages in a normal way. Bending handlers as it does now, leads to all kind of weird side effects. (In reply to comment #5) > But it overwrites not "file->preferences" (where is that anyway?). It > overwrites the default handler for "window->preferences". Sorry, that's what I mean, it provides the behaviour for Window>Preferences. > The problems view should > provide its own command and leave the "window-preferences" handler alone. Or it > could just provide its own preference page and hook up with the preference > pages in a normal way. Bending handlers as it does now, leads to all kind of > weird side effects. But providing one's own handler for a command is *exactly* what commands was designed for. It's not what you expected, but it is what Problems view intented. That being said, I don't see anything wrong with a more general enhancement request. It might be that managing its own preference dialog was a requirement long ago but is not necessary any more. Or it might be that it was intended to migrate to a standard preference page but that work was never scheduled. PW I understand, that you can provide own handler for commands, and that this was indeed the intention. I still think that the way it works in this case is at least disturbing. I debugged quite a while to find out what was going on. Since I have no time to migrate the preference page of the problems view I will note this as a known issue. Managing its own preference dialog is ok, as long as it does not overwrite the default handler. I mean the workflow is that nobody expects the "window->preferences" entry to show the preferences of the currently active workbench part. Interestingly it does not do that (even when the problems view is active), it just that my toolbar button fails to bring up the default preference page (with mine selected). I do not know whats the difference of activating the main menu "windows->preferences" entry and somehwere else. Wow, I also just ran into this. The current behavior is clearly a bug, since I cannot access the Preferences dialog any more (in I20101012-0800). Window > Preferences and its command are not meant to be retargeted. The marker view's "view menu > Preferences" action doesn't need a command (like the other actions in the view menu). (In reply to comment #8) > Wow, I also just ran into this. The current behavior is clearly a bug, since I > cannot access the Preferences dialog any more (in I20101012-0800). > > Window > Preferences and its command are not meant to be retargeted. The marker > view's "view menu > Preferences" action doesn't need a command (like the other > actions in the view menu). +1 Paul, are there other cases where such a clash or undesirable behaviour can occur ? It makes sense in using a different command I guess... Is it possible that this maybe seen as a regression ? What do you think about this ? Thanks. At the time Tod and I did this deliberately. But if it is still causing confusion we could switch it out to a markers.preferences command. PW (In reply to comment #11) > At the time Tod and I did this deliberately. But if it is still causing > confusion we could switch it out to a markers.preferences command. This would be a new command right? I don't see a good reason to have a command for this. No other view has this and there's Ctrl+F10 to open the view menu. If those are really "preferences" then they should end up on a preference page like any other preference. Actually, we should get rid of that preference dialog: we already have a "Columns..." view menu item which allows to select the order and width of the columns. That dialog should also allow to choose which columns are visible instead of putting this into a separate preference dialog. The same has already been requested by bug 231081. This leaves us with the "limit" option which we could move to the 'Configure Contents' dialog which already has all other filtering options. (In reply to comment #12) > (In reply to comment #11) > > At the time Tod and I did this deliberately. But if it is still causing > > confusion we could switch it out to a markers.preferences command. > This would be a new command right? I don't see a good reason to have a command > for this. All of the Marker view menu items are commands. But yes, I was suggesting not hijacking the main preference command and providing a new local one. > Actually, we should get rid of that preference dialog: we already have a > "Columns..." view menu item which allows to select the order and width of the > columns. That dialog should also allow to choose which columns are visible > instead of putting this into a separate preference dialog. The same has already > been requested by bug 231081. Right, in this case markers preferences seem to me to be more "dialog settings". I don't see why they couldn't be combined into on markers "setting" dialog. PW This works with 3.6, 3.7M2a, but is broken again with I20101025-1800. I can confirm Deepak's findings. Was OK up to I20100928-1200, but is broken since I20101005-0800. Yep, see it here too. I'll take a look... Looks like the changes for bug 270007 broke this. (In reply to comment #17) > Looks like the changes for bug 270007 broke this. Will look into this (In reply to comment #18) > (In reply to comment #17) > > Looks like the changes for bug 270007 broke this. Indeed. This should be fixed for M3, given this works in 3.6.x and 3.7 M2. In an effort to remove away from Actions (and registering the actions before using it in the command), the fix was checked in for Bug# 270007. I don't think anything wrong in that one. On the contrary, I don't think its correct for the Markers view to hijack the Preferences command, when it displays the local settings of the view instead of global preferences. I guess the right solution is for the markers view to use a new command or open up the Preference dialog with the a preference page for the Markers view. As a temporary measure, I can revert the patch for 270007, but I'll leave it to Hitesh for fixing this I like the suggestions from Bug 231081. Some code duplication here, the 'Columns...' handler uses ConfigureColumns. Unfortunately, this code is not open to reuse, but does it not sounds more like a requirement for viewers in general ? *** Bug 328688 has been marked as a duplicate of this bug. *** I'm temporarily reverted the changes in Bug# 270007, but even with that, if anyone contributed the Preferences command to a menu/toolbar via commands, they will still face the same issue. This has to be fixed from the Markers view side. (In reply to comment #23) > I'm temporarily reverted the changes in Bug# 270007, Thanks! > This has to be fixed from the Markers view side. I agree. It should be fixed along comment 12. Created attachment 181802 [details] Mock_Up_V01 (In reply to comment #21) (In reply to comment #23) Here is a snippet that tries to behave the way as described above - a demo of what could possibly replace the 2 dialogs. I have also tried to structure it so that one could speculate better if it makes sense to have this as a helper dialog for column-viewers in general. (In reply to comment #25) > Created an attachment (id=181802) [details] > Mock_Up_V01 > > (In reply to comment #21) > (In reply to comment #23) > > Here is a snippet that tries to behave the way as described above - a demo of > what could possibly replace the 2 dialogs. I have also tried to structure it so > that one could speculate better if it makes sense to have this as a helper > dialog for column-viewers in general. Looks good. (*) Can the "Width of the selected column" taken out and replaced with a column in the "Show" table? (*) Why do you need a group for the limit label & text? (In reply to comment #26) > > (*) Can the "Width of the selected column" taken out and replaced with a column > in the "Show" table? > (*) Why do you need a group for the limit label & text? It makes sense since they are related :) One may even suggest including the message area as well...I did not have any particular design in mind; it was meant to be a mock-up for the suggestions discussed yesterday. But, IMHO, with some serious thought one could structure it better. Verified in I20101028-1800 that the original problem is fixed in M3. Is this still targeted for M4? (In reply to comment #29) > Is this still targeted for M4? Actually, it currently works, so basically we can close this bug but make sure that the fix for bug 270007 doesn't break things again. (In reply to comment #30) > (In reply to comment #29) > > Is this still targeted for M4? > Actually, it currently works, so basically we can close this bug but make sure > that the fix for bug 270007 doesn't break things again. Moving to M5 since this is not time-critical work that would have to be done for M4. We'll make sure that the fix for bug 270007 doesn't break things again Verified in I20110124-1800 |