Community
Participate
Working Groups
Build Identifier: M20110210-1200 After upgrading last week to Build id: M20110210-1200 with Eclipse for RCP and RAP Developers - 1.3.2.20110301-1807 the problems started occuring. The Tabbed Propertysheet no longer gets the new selection as the selection changes in the MultiPageEditorPart. This is specific to the Multipage editor as I have no problems with the EditorPart. If I go back to the 1.3.1 version then the Tabbed PropertySheet tracks the MultiPage editor as expected. I am not sure whether it is specific to the Tabbed Property sheet either, but that's what I have been using. I believe the MultiPageSelectionProvider doesn't relay the events, but unfortunately I seem unable to debug it fruther since I am no longer able to see the code for MultiPageEditorPart. This source seems protected by "The JAR of this class file belongs to container 'Plug-in dependencies' which does not allow modifications to source attachements of its entries". A sidenote, previous version had a problem where it was impossible to garbage collect the MultiPage editor because it didn't remove the selectionProvider on close, but I think that's noted in different bug report. Reproducible: Always Steps to Reproduce: 1. Create MultiPageEditorPart 2. Link to Tabbed Property sheet 3. See it doesn't follow the selection in the MultiPageEditorPart
Do all your editor page's selection providers implement IPostSelectionProvider?
(In reply to comment #1) > Do all your editor page's selection providers implement IPostSelectionProvider? No I had not. it works ok after I added the IPostSelectionProvider, thanks! You may close the issue.
After updating the Eclipse installation today, SDK-3.6.2 I am now again able to see the source code, making debugging a lot simpler :-)
(In reply to comment #2) > No I had not. it works ok after I added the IPostSelectionProvider, thanks! Marking as works-for-me
(In reply to comment #4) > (In reply to comment #2) > > No I had not. it works ok after I added the IPostSelectionProvider, thanks! > > Marking as works-for-me I am reopening this as I am not satisfied with the "resolution" of this issue. Plug-ins that worked before 3.6.1 will simply not work anymore after the user upgrades their system to 3.6.2. We need to figure out _why_ this is happening and automatically forward a post selection event if the selection provider doesn't implement IPostSelectionProvider and maybe add an entry into the migration guide (I say "maybe" because this change is in both 3.6.2 and 3.7.0). Per, please provide some code to reproduce this problem.
I have been using this for a modelling framework, but my problem editor is a little hard to pull-out so I will have to come up with an example from scratch if it is necessary. Maybe this gives some clues.... It's basically a MultiPageEditorPart with two pages extending EditorPart who each uses a org.eclipse.jface.viewers.TreeViewer. The multipage editor part implements ITabbedPropertySheetPageContributor and such connects to the Tabbed property Sheet. The EditorPart implements the IPostSelectionProvider / ISelectionProvider and just relays the implementation methods of the TreeViewer's default implementation. As such it works better with the IPostSelectionProvider than the plain ISelectionProvider :-)
(In reply to comment #6) > The EditorPart implements the IPostSelectionProvider / ISelectionProvider and > just relays the implementation methods of the TreeViewer's default > implementation. So what is the difference here? Your nested editor parts used to just implement ISelectionProvider and now it implements IPostSelectionProvider?
(In reply to comment #7) > (In reply to comment #6) > > The EditorPart implements the IPostSelectionProvider / ISelectionProvider and > > just relays the implementation methods of the TreeViewer's default > > implementation. > > So what is the difference here? Your nested editor parts used to just implement > ISelectionProvider and now it implements IPostSelectionProvider? Exactly, it works like before when I changed to the IPostSelectionProvider!
I agree with comment 5. Per it would be good to have a little sample. I suspect that a performance fix we mad in 3.7 (bug 162079) and which we backported to 3.6.2 (bug 332041), reveals a bug in MultiPageSelectionProvider or MultiPageEditorPart.
(In reply to comment #9) > I agree with comment 5. > > Per it would be good to have a little sample. I suspect that a performance fix > we mad in 3.7 (bug 162079) and which we backported to 3.6.2 (bug 332041), > reveals a bug in MultiPageSelectionProvider or MultiPageEditorPart. Ok, I will do. I may be able to do so before the end of this week.
Created attachment 191412 [details] MultiPageEditorSite patch v1 The MultiPageSelectionProvider implements IPostSelectionProvider but may end up not forwarding the selections of its inner editors if those selection providers don't implement IPostSelectionProvider. The fix is to hook up our post selection listeners even if the inner selection providers don't implement IPostSelectionProvider. Should add a test for this.
Created attachment 191416 [details] Code to reproduce the issue This plugin reproduces the issue. To setup: - In the debug workspace create a project and copy paste file \abc\data\data.frt from this plugin. - Open the file with "Fruit Multi-page Editor" - Open the Properties view To duplicate: - Expand any top-level category and select a second level item in the tree. In 3.6 properties are shown in the properties view In 3.6.2 and 3.7 they are only shown when MultiPageEditor switches pages.
To clarify: This issue is the result of the change made in the bug 162079 (Eclipse 3.7) / bug 332041 (Eclipse 3.6.2). The problem: All multi-part editor containing pages that does not support post-selection listeners will stop providing updates to the Properties view. Fix for the immediate problem: Remy’s patch fixes the problem form MPE, similarly to what was done for PageBookView in the bug 162079 comment 20. Concerns: This is the second issue that came from the performance fix for the bug 162079. It makes it clear that that change breaks “selection aggregators” that work on parts that don’t supply post-selection events. Due to the nature of the selection code, unless the person is fully aware of the change in the bug 162079, it will be rather hard for affected Eclipse developers to figure out why their application broke with the update to 3.6.2 / 3.7. As such, my vote goes to rolling back the change made in the bug 162079 in 3.6.2+ and 3.7 streams.
If a selection provider doesn't correctly handle post selection listeners then the bug is clearly there. Having said that, I could agree to revert the performance fix in 3.6.2 again but we should leave it in 3.7 and add a note to the migration guide.
Boris and Paul, what's you take on this?
(In reply to comment #15) > Boris and Paul, what's you take on this? I agree with Dani. We should roll it back (in 3.6.2+) but keep it in 3.7. We should also mention it soon on cross-project-issues so the release train is aware. PW
I can take this bug if you want, given I broke it :-(
(In reply to comment #17) > I can take this bug if you want, given I broke it :-( I'm planning to open a new bug next week (for the MPSP problem itself) and then leave this one open for the discussion of rollback/migration guide/what-not. I also wanted to add a test for the MPSP selection problem and probably another test to check that the 'Properties' view reacts properly.
Good. Looks like you want to drive/own this ;-)
Opened bug 341219 for the changes required for MultiPageEditorSite and bug 341220 for an update to the migration guide.
Should I rollback all the changes from attachment 184778 [details] or just the changes to PropertySheet? I would like to avoid changing excessive amounts of code if necessary.
(In reply to comment #21) > Should I rollback all the changes from attachment 184778 [details] [diff] or just the changes to > PropertySheet? I would like to avoid changing excessive amounts of code if > necessary. I agree: we should only change back to use the normal selection listener. However, Remy it would be good to review the other changes again - just to be sure.
Created attachment 192282 [details] PropertySheetPage patch v1 Marking attachment 191412 [details] as obsolete as that bug has been fixed by bug 341219. Thank you very much for the bug report, Per! (In reply to comment #22) > However, Remy it would be good to review the other changes again - just to be > sure. There doesn't seem to be anything suspicious here. The changes to TabbedPropertySheetPageTextTest have been modified to be on a timeout instead so it shouldn't get into an infinite loop even if things go wrong. The changes for PageBookView are legitimate given that the selection provider does indeed implement IPostSelectionProvider but may not actually forward any such events out depending on what page is currently shown.
Boris, please review attachment 192282 [details]. Please refer to comment 13 for context.
After discussing with Remy, I can give my +1 for rolling back the change in PropertySheet.java in 3.6.2.
(In reply to comment #23) > Created attachment 192282 [details] > PropertySheetPage patch v1 Patch released to R3_6_maintenance branch. Thank you to all parties involved and thank you again Per for reporting this bug.
Whoops. :)
