Bug 198101 - [DataBinding] ComboObservableValue fails to clear cached value
Summary: [DataBinding] ComboObservableValue fails to clear cached value
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.4 M3   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-07-27 09:22 EDT by Katarzyna Marsza³ek CLA
Modified: 2007-12-12 11:27 EST (History)
2 users (show)

See Also:

Attachments
patch against 3.3 maintenance branch (1.83 KB, patch)
2007-10-22 15:43 EDT, Robert Personen 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 Katarzyna Marsza³ek CLA 2007-07-27 09:22:08 EDT 
ComboObservableValue fails to update cached currentValue during the doSetValue(...) call. As the result, event may not fire if the next combo selection happens to be the same as cached value. Proposed change (new lines end with // FIX):


                public void modifyText(ModifyEvent e) {
                    if (!updating) {
                        String oldValue = currentValue;
                        currentValue = ComboObservableValue.this.combo.getText();
                        fireValueChange(Diffs.createValueDiff(oldValue, currentValue));
                    } else // FIX
                        currentValue = ComboObservableValue.this.combo.getText(); // FIX
Comment 1 Boris Bokowski CLA 2007-09-18 13:19:34 EDT 
Moving to M3.
Comment 2 Robert Personen CLA 2007-10-17 11:46:06 EDT 
This bug occurs whenever the model is changed.  Change the value of the model will be reflected in the widget but not cached in the Observable.  The Observable's concept of "current" or "previous" value is now out of sync with the Combo widget.  

Assuming that the ComboObservableValue's currentValue is supposed to mirror what the widget shows, a better fix would be to change doSetValue() from:


} finally {
    updating = false;
}
fireValueChange(Diffs.createValueDiff(oldValue, combo.getText()));


to:


} finally {
    // ensure that the Observable is in sync with the widget
    currentValue = combo.getText();
    updating = false;
}
fireValueChange(Diffs.createValueDiff(oldValue, currentValue));


This should cause any modification from the model (including failed setting of the combo in the case of read only combo list) to be mirrored in the cached currentValue.
Comment 3 Brad Reynolds CLA 2007-10-17 18:46:52 EDT 
Is anyone interested in contributing[1] a patch?

[1] http://wiki.eclipse.org/JFace_Data_Binding/How_to_Contribute
Comment 4 Robert Personen CLA 2007-10-22 15:43:44 EDT 
Created attachment 80899 [details]
patch against 3.3 maintenance branch

This fixes the bug against 3.3 maintenance branch.  There are changes in HEAD that differs between CCombo and Combo so I made no attempt to create a patch against it.
Comment 5 Boris Bokowski CLA 2007-10-29 00:51:56 EDT 
The patch did not apply cleanly to HEAD, but I was able to make the same kind of change. Thanks for your help!
Comment 6 Boris Bokowski CLA 2007-10-29 13:11:21 EDT 
Fix released to HEAD.
Comment 7 Boris Bokowski CLA 2007-12-12 11:27:23 EST 
Verified by code inspection using I20071211-0010.