Bug 65464 - [Problems] (regression) Not possible to add contributions to ProblemMarker or ConcreteMarker without referencing internal classes
Summary: [Problems] (regression) Not possible to add contributions to ProblemMarker or...
Status: CLOSED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P2 major (vote)
Target Milestone: 3.0 RC2   Edit
Assignee: Nick Edgar CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 53217
Blocks:
  Show dependency tree
 
Reported: 2004-06-02 18:53 EDT by Lawrence Mandel CLA
Modified: 2006-05-01 14:20 EDT (History)
7 users (show)

See Also:


Attachments
Patch for ConcreteMarker.java (1.45 KB, patch)
2004-06-03 03:05 EDT, Gunnar Wagenknecht CLA
no flags Details | Diff
Patch to IDE using different selection provider (3.89 KB, patch)
2004-06-04 23:28 EDT, Nick Edgar CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lawrence Mandel CLA 2004-06-02 18:53:41 EDT
I have a plugin which assigns a popup action to problem IMarkers. This
worked fine in Eclipse 2.1.x but in Eclipse 3.0, as you know, the problems
view has been separated from the tasks view. In doing so a ProblemMarker
class was created which extends the ConcreteMarker class. Neither of these
classes implement IMarker. This is a regression in Eclipse 3.0. A couple 
questions.

1. Why don't either of these classes implement IMarker? Was this a design
decision or is this an accidental omission?

2. If these classes can't implement IMarker (for whatever reason) can a
public interface be created so I don't have to register my popup action
against an internal class?

I posted this to the platform UI dev mailing list and it was suggested by John 
Arthorne to have ConcreteMarker implement IAdaptable and adapt to IMarker so I 
can register object contributions against IMarkers.

Thanks.
Comment 1 Pat McCarthy CLA 2004-06-02 20:55:04 EDT
Ouch.  Was just trying this out too.

The suggested resolution might work fine -the only hit then would be we'd have 
to add the adaptable="true" to the objectClass=....IMarker entries. My 2.1.1 
test says I can contribute without that entry.

Not sure you can hold this back from RC2... the alternative is a bit messy.
Looks like we'd target internal classes as Lawrence says, then we'd have to 
cast to that class/ConcreteMarker, then getMarker(), then go on.  But the 
other options for the marker contribution may not fly (enablement by attribute 
value).
Comment 2 Gunnar Wagenknecht CLA 2004-06-03 00:57:10 EDT
Serverity should be higher because this is a breaking change IMHO which is not 
documented. At least some documentation must be added to the migration guide 
with a workaround. Better would be a fix.
Comment 3 Gunnar Wagenknecht CLA 2004-06-03 03:05:33 EDT
Created attachment 11522 [details]
Patch for ConcreteMarker.java

This simple patch enables ConcreteMarker and all sub classes to adapt to
IMarker-
Comment 4 Gunnar Wagenknecht CLA 2004-06-03 03:15:30 EDT
Adding dependency to bug 53217 because this bug covers some limit of 
the "adaptable" attribute.
Comment 5 Michael Van Meekeren CLA 2004-06-03 09:07:01 EDT
nick could you review the patch?
Comment 6 Nick Edgar CLA 2004-06-04 23:28:13 EDT
Created attachment 11626 [details]
Patch to IDE using different selection provider

This patch takes a different approach: it passes a different selection provider
to registerContextMenu, which maps from ConcreteMarker to the wrapped IMarker,
so the menu extension mechanism sees IMarkers, not ConcreteMarkers.

This will address the case where adaptable="true" is not specified.

This requires an API addition: TableView.registerContextMenu(IMenuManager) and
an override of this method in MarkerView.

Jeem?
Comment 7 Gunnar Wagenknecht CLA 2004-06-05 03:00:28 EDT
Interesting. Does this also work for actions contributed to the main window 
menu and the toolbar?
Comment 8 Nick Edgar CLA 2004-06-07 11:17:42 EDT
No, the above patch will only work for contributions adding to the context menu.
Comment 9 Jim des Rivieres CLA 2004-06-07 14:50:19 EDT
API addition? TableView is in org.eclipse.ui.views.markers.internal; 
MarkerView is in org.eclipse.ui.views.markers.internal. So there is no API 
exposure here.
Comment 10 Nick Edgar CLA 2004-06-07 16:17:53 EDT
Right.  Need more sleep.
Comment 11 Nick Edgar CLA 2004-06-08 16:35:13 EDT
There's actually a more concise fix.
In TableView.createPartControl, replace:
		viewer.getControl().setMenu(menu);
		getSite().registerContextMenu(mgr, viewer);

with:
		viewer.getControl().setMenu(menu);
		getSite().registerContextMenu(mgr, getSelectionProvider());
		
		getSite().setSelectionProvider(getSelectionProvider());
		
The marker views already had a selection provider separate from the view, which
did the mapping between IMarker and ConcreteMarker.

This also fixes the action set case (comment #7).  The views weren't previously
registering their selection provider on the site.
Comment 12 Nick Edgar CLA 2004-06-09 10:48:34 EDT
Verified against binaries of I20040609-0800.
Comment 13 Lawrence Mandel CLA 2006-05-01 14:20:13 EDT
I've verified this fix. Closing bug.