Bug 29398

Comment 1 Dejan Glozic 2003-01-19 21:00:39 EST

The problem is fairly complicated. PDE editors are multi-page - they implement 
IEditorPart but the source page itself is an IEditorPart (also ITextEditor and 
ITextEditorExtension).

When BookmarkRulerAction is registered using the viewerContribution element in 
popupMenus extension point, this action reacts to top-level editor switching 
only. When the active editor changes, it tests if the new editor is ITextEditor 
and also ITextEditorExtension and does nothing if it isn't.

Kai, this is a problem for use because text editor is just one of the pages for 
us. How can we contribute actions to the vertical ruler when our top-level 
editor is not a text editor, but one of the pages is?

Comment 2 Wassim Melhem 2004-04-10 03:41:34 EDT

*** Bug 57673 has been marked as a duplicate of this bug. ***

Comment 3 Wassim Melhem 2004-05-03 02:36:56 EDT

Fixed in the new editor.

Comment 4 Nitin Dahyabhai 2005-06-24 15:10:39 EDT

Broken on all 3 pages (build.properties, plugin.xml, manifest.mf) in the editor
in 3.1rc3.

Perhaps the AbstractRulerActionDelegate should ask the editor part for an
ITextEditor as an adapter?  This could solve the problem for other multi-page
editors with a single source page.

Comment 5 Wassim Melhem 2005-07-13 09:37:13 EDT

This functionality was lost some time toward the end of the 3.1 cycle.  Not 
sure exactly when.

Dani, do you know what could have happened here?

Comment 6 Dani Megert 2005-07-13 10:49:19 EDT

It currently works via menu but not via ruler. AFAIK we didn't change any code
in that area, and I doubt it ever worked from the ruler when looking at the code.

>Perhaps the AbstractRulerActionDelegate should ask the editor part for an
>ITextEditor as an adapter?  This could solve the problem for other multi-page
>editors with a single source page.
The PDE editor has more than one source page.

Comment 7 Nitin Dahyabhai 2005-07-13 11:35:48 EDT

Changing the severity from an Enhancement to Normal--the action exists and shows
up, it just doesn't work. 

Comment 8 Wassim Melhem 2005-07-20 13:15:39 EDT

Konrad, please investigate for 3.1.1

Comment 9 Konrad Kolosowski 2005-07-26 17:33:47 EDT

I have done some investigation.  It worked in M6, broken since M7.

In M6 MultipageEditorSite used to have:

    public void registerContextMenu(String menuID, MenuManager menuMgr,
            ISelectionProvider selProvider) {
        if (menuExtenders == null) {
            menuExtenders = new ArrayList(1);
        }
        menuExtenders.add(new PopupMenuExtender(menuID, menuMgr, selProvider,
                editor));
    }

where editor was ManifestSourcePage when opening manifest editor.
Since M7 till now, the method reads:

    public void registerContextMenu(String menuID, MenuManager menuMgr,
            ISelectionProvider selProvider) {
        getMultiPageEditor().getSite().registerContextMenu(menuID, menuMgr, 
selProvider);
    }
where getSite() resolves to PartSite for ManifestEditor which is the outder 
editor so does not implement ITextEditor.


If, in org.eclipse.ui.workbench plug-in, I replace MultiPageEditorSite with 
the previous revision (1.15) then bookmarks and tasks work again.

Nick, I don't quite understand the reason for the change in 
MultiPageEditorSite.  Can this be corrected on the UI side, or is there 
another way PDE editors should register context menu?

Thanks.

Comment 10 Nick Edgar 2005-07-28 10:36:51 EDT

See bug 93998 for the rationale for this change.
In retrospect, we should not have done this, as it worked this way in 3.0. The
inconsistency was not due to the change for bug 68938 as I had assumed, it just
propagated the same inconsistency.

So it looks like we'll need to roll back the change for bug 93998 and live with
the inconsistency, and we can do this for 3.1.1.

Comment 11 Konrad Kolosowski 2005-07-28 10:41:23 EDT

This will be great for 3.1.1.  Thanks Nick.

Comment 12 Douglas Pollock 2005-08-02 16:13:43 EDT

Created attachment 25582 [details]
Patch to "MultiPageEditorSite"

This simply reverts the fix for Bug 93998.  I believe MultiPageEditorSite is
the only class affected.

Comment 13 Douglas Pollock 2005-08-02 16:30:20 EDT

Fixed in both the 3.2 stream and the 3.1.1 stream. 

Comment 14 Karice McIntyre 2005-09-26 11:45:04 EDT

Verified can create a bookmark using menu item (but not by double clicking in 
ruler) in manifest.mf, plugin.xml, and build.properties.
Using 3.1.1 20050923-1430 running in simplified chinese, using NLpack1-eclipse-
SDK-M20050816-1400-win32.zip.

Comment 15 Karice McIntyre 2005-09-26 12:32:08 EDT

I should not have marked this as verified as it hasn't yet been verified in 3.2

Comment 16 Karice McIntyre 2005-09-26 12:32:34 EDT

Verified in 3.1.1 but not 3.2 yet

Comment 17 Douglas Pollock 2005-09-26 13:48:06 EDT

Verified on 3.1.1 RC2 and 3.2 M2 using GTK+ 2.6.8.