Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349297 - [DataBinding] WorkbenchProperties.SelectionServiceListener ignores arguments given to selectionChanged() when delegating to NativePropertyListener.fireChanged()
Summary: [DataBinding] WorkbenchProperties.SelectionServiceListener ignores arguments ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.8 M3   Edit
Assignee: Dani Megert CLA
QA Contact: Matthew Hall CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 357568 358920 362709
  Show dependency tree
 
Reported: 2011-06-14 05:52 EDT by thammers CLA
Modified: 2011-11-08 08:31 EST (History)
7 users (show)

See Also:


Attachments
Fix (1.44 KB, patch)
2011-08-25 01:30 EDT, Dani Megert CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description thammers CLA 2011-06-14 05:52:20 EDT
Build Identifier: I20110603-0909

The inner class SelectionServiceListener always calls its inherited method fireChanged() with arguments (null, null) ignoring the part and selection passed to selectionChanged(). The super class NativePropertyListener creates a new SimplePropertyEvent in fireChanged() and passes through the null values. This results in an IllegalArgumentException ("null source") in the constructor of SimplePropertyEvent/EventObject. Due to this fact the provided WorkbenchProperties are unusable.

Reproducible: Always
Comment 1 Paul Webster CLA 2011-06-14 08:18:48 EDT
Boris?

PW
Comment 2 Boris Bokowski CLA 2011-06-14 09:01:18 EDT
(In reply to comment #0)
> Due to this fact the provided WorkbenchProperties are unusable.

That's too bad, sorry about that! Unfortunately, this bug report is coming in too late for the 3.7 release which is essentially done now. Marking for 3.7.1.
Comment 3 Dani Megert CLA 2011-08-25 01:30:50 EDT
Created attachment 202119 [details]
Fix
Comment 4 Matthew Hall CLA 2011-09-20 02:02:47 EDT
Who is maintaining WorkbenchProperties?
Comment 5 Dani Megert CLA 2011-09-20 02:23:50 EDT
(In reply to comment #4)
> Who is maintaining WorkbenchProperties?

Since it's data binding, I have you on the list ;-). Why?
Comment 6 Matthew Hall CLA 2011-09-20 02:28:50 EDT
I helped review the initial submission for correctness but didn't know I would be maintaining it. I'm fine with that, but first I need to know where it is in CVS.

Also, what are you doing up so late? ;-)
Comment 7 Dani Megert CLA 2011-09-20 02:39:02 EDT
(In reply to comment #6)
> I helped review the initial submission for correctness but didn't know I would
> be maintaining it.
If that's not the case you should speak to Paul or Eric.

> I'm fine with that, but first I need to know where it is in
> CVS.
Platform UI code is no longer in CVS. It has been moved to Git, see http://wiki.eclipse.org/Platform_UI/Git for details.

> Also, what are you doing up so late? ;-)
I'm sitting in Zurich :-)


Regarding this bug: I plan to put it into 3.7.2. If you could quickly review it that would be great.
Comment 8 Matthew Hall CLA 2011-09-20 20:17:50 EDT
(In reply to comment #7)
> Platform UI code is no longer in CVS. It has been moved to Git, see
> http://wiki.eclipse.org/Platform_UI/Git for details.

To borrow an Adam Sandler quote: "Something that should have been brought to my attention YESTERDAY!"

I've been on hiatus for a while and didn't know that DataBinding had been migrated to Git. Which means the patches I committed yesterday to CVS need to be committed to Git. Hooray.
Comment 9 Dani Megert CLA 2011-09-21 02:21:45 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > Platform UI code is no longer in CVS. It has been moved to Git, see
> > http://wiki.eclipse.org/Platform_UI/Git for details.
> 
> To borrow an Adam Sandler quote: "Something that should have been brought to my
> attention YESTERDAY!"

It has been announced on the platform-ui developer mailing list several times in Juliy, e.g. http://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg04971.html.
Comment 10 Matthew Hall CLA 2011-09-21 12:23:54 EDT
Due to an extended delay getting approval paperwork from my new employer to remain an Eclipse committer, EF removed me from those mailing lists. Unfortunately, I was not reinstated to them once the paperwork went through.

I'm actually still a little torqued about how that was handled from the EF side.</rant>

I've looked around on the committer portal but have not found a way to sign up for the mailing lists.
Comment 11 Dani Megert CLA 2011-09-22 01:50:32 EDT
> I've looked around on the committer portal but have not found a way to sign up
> for the mailing lists.
Just register here: https://dev.eclipse.org/mailman/listinfo/platform-ui-dev
Comment 12 Matthew Hall CLA 2011-09-24 01:17:54 EDT
On its face, the code looks correct.

Do we have a use case to verify it actually fixes the problem?
Comment 13 Dani Megert CLA 2011-09-26 11:41:58 EDT
R3_development commit 39bea189cb93b09fa4a5aab4a7189a586d22b503


I cherry picked it in my R4_dev workspace but so far I get an error when trying to push. Grrr!

I'll clone this bug for 3.7.2 once I've sorted out the push issue.
Comment 14 Dani Megert CLA 2011-09-26 11:54:18 EDT
> I cherry picked it in my R4_dev workspace but so far I get an error when trying
> to push. Grrr!
Restart fixed the issue.

R4_development commit: 07354e7cef1b36f0f71f9fd1795506ed802f5a67

> I'll clone this bug for 3.7.2 once I've sorted out the push issue.
See bug 358920.
Comment 15 Dani Megert CLA 2011-10-26 04:51:01 EDT
Verified in 3.8-I20111025-1800 and 4.2-I20111018-2000.
Comment 16 Dani Megert CLA 2011-11-08 08:31:21 EST
(In reply to comment #13)
> R3_development commit 39bea189cb93b09fa4a5aab4a7189a586d22b503
The correct change id is: 5588a2feb355998573c4f56bd103ad4d1c5c3eb4