Bug 223808 - [PropertiesDialog] Properties dialog page hierarchy screwed up
Summary: [PropertiesDialog] Properties dialog page hierarchy screwed up
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.4 M6   Edit
Assignee: Kim Horne CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-03-25 07:36 EDT by Markus Keller CLA
Modified: 2008-03-28 11:54 EDT (History)
3 users (show)

See Also:

Attachments
Fix (9.49 KB, patch)
2008-03-25 15:59 EDT, Francis Upton IV CLA 		no flags Details | Diff
Revised per suggestion (9.47 KB, patch)
2008-03-25 19:41 EDT, Francis Upton IV CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2008-03-25 07:36:14 EDT 
I20080325-0100

Compare e.g. contents of the tree in the Properties dialog for org.eclipse.core.expressions from CVS in I20080318-0800 (OK) and I20080325-0100 (multiple child pages shown as top-level pages).
Comment 1 Kim Horne CLA 2008-03-25 09:55:49 EDT 
The most likely candidate for this is the fix for bug 219273
Comment 2 Francis Upton IV CLA 2008-03-25 11:04:56 EDT 
I will have a look at this.
Comment 3 Francis Upton IV CLA 2008-03-25 15:59:41 EDT 
Created attachment 93482 [details]
Fix

Problem happened because the RegistryPageContributor and PropertyPageManager assumed the contributions were in such an order that the parent contributions were before all children.  This is not necessarily true.

Changed the code to work regardless of the order.  Also cleaned up a method name to be less misleading.  Maybe even completely leading.
Comment 4 Francis Upton IV CLA 2008-03-25 16:00:50 EDT 
Oh and ran UI and RCP tests against 0325-0100 as well as testing properties and preferences by hand.
Comment 5 Kim Horne CLA 2008-03-25 17:54:06 EDT 
I think the only issue I have is the existence of fixPageParent in the IPropertyPageContributor.  It seems like there is only one sensible implementation of this method and given that there's only one caller we can probably safely inline the call in PropertyPageContributor.contribute().
Comment 6 Francis Upton IV CLA 2008-03-25 19:41:14 EDT 
Created attachment 93513 [details]
Revised per suggestion

The reason I did not do this initially was I thought there was some sort of Grand Architecture that had the PropertyPageContributor not be aware of the class of the property pages.  And I wanted to be respectful of said G.A.

I made the PropertyPageContributor assume the pages were PreferenceNodes.

Retested everything.
Comment 7 Kim Horne CLA 2008-03-26 10:03:35 EDT 
Looks good to me.  It's going into HEAD.  I did add one thing, however  - you neglected to add your copyright notice to one of the files you changed.  I just copied it from another one you modified.
Comment 8 Kim Horne CLA 2008-03-27 09:11:12 EDT 
Verified in I20080327-0100
Comment 9 Francis Upton IV CLA 2008-03-28 11:54:17 EDT 
See 224653 for the refactoring opportunity here.