Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 65464

Summary:

[Problems] (regression) Not possible to add contributions to ProblemMarker or ConcreteMarker without referencing internal classes

Product:

[Eclipse Project] Platform

Reporter:

Lawrence Mandel <lmandel>

Component:

Assignee:

Nick Edgar <n.a.edgar>

Status:

CLOSED FIXED

QA Contact:

Severity:

major

Priority:

CC:

daniel_megert, danjou, danrubel, gunnar, jeem, John_Wiegand, patmc

Version:

3.0

Target Milestone:

3.0 RC2

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

53217

Bug Blocks:

Attachments:

Description	Flags
Patch for ConcreteMarker.java	none
Patch to IDE using different selection provider	none

Description Lawrence Mandel

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

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

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

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

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

2004-06-03 09:07:01 EDT

nick could you review the patch?

Comment 6 Nick Edgar

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

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

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

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

2004-06-07 16:17:53 EDT

Right.  Need more sleep.

Comment 11 Nick Edgar

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

2004-06-09 10:48:34 EDT

Verified against binaries of I20040609-0800.

Comment 13 Lawrence Mandel

2006-05-01 14:20:13 EDT

I've verified this fix. Closing bug.