Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 289599 - [Preferences] PreferenceStore.setToDefault fires redundant PropertyChangeEvents
Summary: [Preferences] PreferenceStore.setToDefault fires redundant PropertyChangeEvents
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M3   Edit
Assignee: Oleg Besedin CLA
QA Contact: Oleg Besedin CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 289556
  Show dependency tree
 
Reported: 2009-09-16 09:06 EDT by Pawel Pogorzelski CLA
Modified: 2009-10-27 14:11 EDT (History)
2 users (show)

See Also:

Attachments
Patch_v01 (1.52 KB, text/plain)
2009-09-16 09:06 EDT, Pawel Pogorzelski CLA 		ob1.eclipse: iplog+
Details
Patch_v01B (2.35 KB, patch)
2009-09-29 06:24 EDT, Pawel Pogorzelski CLA 		ob1.eclipse: iplog+
Details | Diff
A bit modified Patch v01 (1.50 KB, patch)
2009-09-29 17:10 EDT, Oleg Besedin CLA 		no flags Details | Diff
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 Pawel Pogorzelski CLA 2009-09-16 09:06:49 EDT 
Created attachment 147303 [details]
Patch_v01

This happens any time the method is called. I'd expect the event to be fired
only if the actual state changes.
Comment 1 Pawel Pogorzelski CLA 2009-09-25 11:02:32 EDT 
Could someone provide a target? The problem is pretty straightforward as well as the attached patch.
Comment 2 Oleg Besedin CLA 2009-09-25 11:10:39 EDT 
Sure, 36M3 for now. Do you by chance have a JUnit?
Comment 3 Pawel Pogorzelski CLA 2009-09-29 06:24:13 EDT 
Created attachment 148311 [details]
Patch_v01B
Comment 4 Pawel Pogorzelski CLA 2009-09-29 06:32:16 EDT 
Oleg, I attached the test in the previous comment. It only gives the idea, you should probably relocate and enhance it.
Comment 5 Oleg Besedin CLA 2009-09-29 17:10:21 EDT 
Created attachment 148370 [details]
A bit modified Patch v01

Thank you, it is much easier to check things once there is a test.

The patch and JUnit look very good. One thing, it would seem to be a bit safer to check 
 properties.containsKey(name)
rather then
 properties.get(name) != null

It should be about the same as PreferenceStore does not allow null values, but seems just a tiny bit safer in case somebody manages to put a null value in it.

What do you think about it?
Comment 6 Pawel Pogorzelski CLA 2009-09-30 04:16:29 EDT 
(In reply to comment #5)

> What do you think about it?

Sure, looks neater.
Comment 7 Oleg Besedin CLA 2009-09-30 13:51:20 EDT 
Patches applied to CVS Head. Thanks Pawel!
Comment 8 Oleg Besedin CLA 2009-10-27 14:11:48 EDT 
Verified that JUnit passed in I20091026-1800 and passes locally on I20091027-0100 build.