Two MVP bugs

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Two MVP bugs

Chris Uppal-3
Bug1: I've just started using Ian's RadioButtonGroup button (thanks Ian!)
with -- naturally -- a bunch of RadioButtons.  I hook his changed notification
(#radioButtonSelection:) which is itself triggered by the #actionPerformed of
the RadioButtons themselves.  In my handler I want to find out which button is
now selected.  This is what breaks because the system still "thinks" that the
old button is selected.  I believe that the problems is caused by
RadioButton>>command:id: which super-sends the notification *before* it clears
the sibling buttons.  If that method is rearranged to put the super-send after
the test against BN_CLICKED then it all works perfectly.

Bug2:  I'm not sure how to describe this, it's a screw-up somewhere in the way
that ValuePresenters do changed notification, and how they handle #value during
that notification (which causes a re-synch with the View).  I don't have a fix
for this one, but to reproduce it, try:

    bp := BooleanPresenter showOn: true asValue.

    "now evaluate this, the checkmark should vanish correctly"
    bp value: false.

    "now install a handler for the changed notification"
    handler := [Transcript display: bp value; cr].
    bp when: #valueChanged send: #value to: handler.

    "now evaluate this, the checkmark does not reappear, and there
    are two messages written to the Transcript"
    bp value: true.

The workaround is to ensure that you don't access the Presenter's #value
directly:

    aPresenter value.

but instead go via the model:

    aPresenter model value.

I'm not sure if this problem is restricted to BooleanPresenters, that's the
only place I've noticed it so far.

    -- chris


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Ian Bartholomew-19
Chris,

> Bug2:  I'm not sure how to describe this, it's a screw-up somewhere
> in the way that ValuePresenters do changed notification, and how they
> handle #value during that notification (which causes a re-synch with
> the View).  I don't have a fix for this one, but to reproduce it, try:

I think this is known about - see my post on 23/11/2000 entitled
ValuePresenter>>value.  There were no responses in the newsgroup but I did
have some e-mail correspondence with Andy on the subject.  I *think* (I
can't find the actual e-mail at the moment) that the problem was that any
fix would break some existing behaviour - TextEdits seem to ring a bell.

> I'm not sure if this problem is restricted to BooleanPresenters,
> that's the only place I've noticed it so far.

I think it affects any Presenter that wraps a Windows control - I remember
coming across the problem again  when trying to work with NumberPresenters
and Spinner controls.

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Chris Uppal-3
Ian,

> > Bug2:  I'm not sure how to describe this, it's a screw-up somewhere
> > in the way that ValuePresenters do changed notification, and how they
> > handle #value during that notification (which causes a re-synch with
> > the View).  I don't have a fix for this one, but to reproduce it, try:
>
> I think this is known about - see my post on 23/11/2000 entitled
> ValuePresenter>>value.  There were no responses in the newsgroup but I did
> have some e-mail correspondence with Andy on the subject.  I *think* (I
> can't find the actual e-mail at the moment) that the problem was that any
> fix would break some existing behaviour - TextEdits seem to ring a bell.

Right.  Thanks Ian.  That does look like the same bug.  I hope Andy decides
that there /is/ a way of fixing it, though, because as it stands
ValuePresenter>>value is useless (in that it can have very undesirable side
effects) and might as well be deprecated.  (But then, if it's deprecated, why
not just fix it instead, and live with the behaviour change ?)

FWIW, I tried a few other ValuePresenters, NumberPresenter seems OK (with its
default view), but DatePresenter (again with default view) is also buggy.

I'm also starting to wonder if it, or something like it, is behind another bug
that I've not been able to pin down yet.  One of my tools uses a range of
dates, each of which can be nil.  The View presents the range as a pair of
DatePresenters, each connected to a ValueAspectAdapter.  Under some
circumstances, that I've never been able to reproduce, it takes 2 toggles of
the checkmark before the changes (toggling the internal values between a real
date and nil) take effect.  Thereafter they work fine...  Puzzling, but perhaps
a relative of the same bug.

    -- chris


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Bill Schwab-2
Chris, Ian, Andy,

> FWIW, I tried a few other ValuePresenters, NumberPresenter seems OK (with
its
> default view), but DatePresenter (again with default view) is also buggy.
>
> I'm also starting to wonder if it, or something like it, is behind another
bug
> that I've not been able to pin down yet.  One of my tools uses a range of
> dates, each of which can be nil.  The View presents the range as a pair of
> DatePresenters, each connected to a ValueAspectAdapter.  Under some
> circumstances, that I've never been able to reproduce, it takes 2 toggles
of
> the checkmark before the changes (toggling the internal values between a
real
> date and nil) take effect.  Thereafter they work fine...  Puzzling, but
perhaps
> a relative of the same bug.

Could this have something to do with idle time and/or a need to do things
non-synchronously with certain native controls?  Have you tried it from a
unit test?  If there is a way to reliably detect the failure scenario
(visual-only things being _very_ troublesome), a unit test will push things
fast enough to expose many weaknesses.

Have a good one,

Bill

--
Wilhelm K. Schwab, Ph.D.
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Chris Uppal-3
Bill,

> > I'm also starting to wonder if it, or something like it, is behind
> > another bug that I've not been able to pin down yet.  One of my tools
> > uses a range of dates, each of which can be nil.  The View presents the
> > range as a pair of DatePresenters, each connected to a
> > ValueAspectAdapter.  Under some circumstances, that I've never been
> > able to reproduce, it takes 2 toggles of the checkmark before the
> > changes (toggling the internal values between a real date and nil) take
> > effect.  Thereafter they work fine...  Puzzling, but perhaps a relative
> > of the same bug.
>
> Could this have something to do with idle time and/or a need to do things
> non-synchronously with certain native controls?

I don't /think/ so.  It's too deterministic for that -- after the app starts,
if the bug is going to manifest at all, it takes /exactly/ two clicks on the
date's checkmarks for the changes to "stick", thereafter they work properly.
The difficulty in reproducing it is that it doesn't happen every time the app
starts (though it does happen most times) and that it doesn't happen at all
under the debugger.  I'm wondering if the "Heisenbug" aspect is caused by the
debugger implicitly calling #value as a result of the various #printString-s.
Hmm...  I'll have to try hacking around with #debugPrintString, see if that
makes the problem debuggable.

Blair, if you are reading.  I think it's time to resurrect a suggestion that
was made some time ago (I forget who first suggested it): I think it would be
(occasionally) invaluable if the debugger could optionally run in a mode it
doesn't display anything that hasn't been /explicitly/ asked for (i.e. doesn't
implicitly run any "application" code).

    -- chris


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Blair McGlashan
In reply to this post by Chris Uppal-3
"Chris Uppal" <[hidden email]> wrote in message
news:[hidden email]...
> Bug1: I've just started using Ian's RadioButtonGroup button (thanks Ian!)
> with -- naturally -- a bunch of RadioButtons.  I hook his changed
notification
> (#radioButtonSelection:) which is itself triggered by the #actionPerformed
of
> the RadioButtons themselves.  In my handler I want to find out which
button is
> now selected.  This is what breaks because the system still "thinks" that
the
> old button is selected.  I believe that the problems is caused by
> RadioButton>>command:id: which super-sends the notification *before* it
clears
> the sibling buttons.  If that method is rearranged to put the super-send
after
> the test against BN_CLICKED then it all works perfectly.

Actually I don't think that RadioButton itself should be toggling the value
of the other buttons at all. That responsibility should be down to whatever
presenter is grouping the radio buttons and implementing the "group"
behaviour. A historical design error (or at least omission) is that we
didn't provide a grouping presenter for the buttons, and hence the
RadioButton view itself does a little too much. For Dolphin 6 we have a
RadioButtonSetPresenter which does indeed manage the buttons so that only
one of them can be selected at any one time, and the
RadioButton>>command:id: method has been removed. For Dolphin 5.1.5, though,
we can apply a temporary patch as you suggest, thanks.

>
> Bug2:  I'm not sure how to describe this, it's a screw-up somewhere in the
way
> that ValuePresenters do changed notification, and how they handle #value
during
> that notification (which causes a re-synch with the View).  I don't have a
fix
> for this one, but to reproduce it, try:

As Ian suggests, this is a known timing issue related to the state of the
MVP triad when the #valueChanged event is fired. Essentially the value in
the model and that in the view may not have been synchronised at that time.
Since ValuePresenter>>value always synchronises the current view value, this
may overwrite any pending change in the model's value. The discussion around
the defect report (#53) is quite extensive, but the conclusion was that the
"right" thing to do would be to remove the automatic synchronisation of the
view value back to the model when the presenter's value is accessed.
Unfortunately this breaks a lot of existing code that relies on being able
to access the current view value before it has been synchronised back to the
model due in some other way (either by explicit request, as a result of
losing focus, or, in the case of a text field, because the #updatePerChar
aspect has been enabled). This use case turns up pretty frequently in
#queryCommand: and other validation code. Most of these cases should
probably be using #updatePerChar (or similar), but it was considered too
heavyweight to make that the default, and if not the default then the "new"
value in the view itself being ignored when accesssing the #value of the
presenter would be a frequent surprise.

At this time we don't have any other solution for this issue, except to make
it clear that ValuePresenter>>value should be avoided inside any event
handler for that presenter's #valueChanged event.

Thanks for the reports

Blair


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Chris Uppal-3
Blair McGlashan wrote:

> > Bug2:  I'm not sure how to describe this, it's a screw-up somewhere in
> > the way that ValuePresenters do changed notification, and how they
> > handle #value during that notification (which causes a re-synch with
> > the View).  I don't have a fix for this one, but to reproduce it, try:
>
> As Ian suggests, this is a known timing issue related to the state of the
> MVP triad when the #valueChanged event is fired
> [...]
> At this time we don't have any other solution for this issue, except to
> make it clear that ValuePresenter>>value should be avoided inside any
> event handler for that presenter's #valueChanged event.

The problem with that -- and the reason I suggested that ValuePresenter>>value
was essentially useless -- is that the place that #value is sent is quite
likely to be several layers of software away from anywhere that knows it is
in a #valueChanged event.

Still, I agree that it's tricky to find a solution that wouldn't break the
existing code you mention (though, with a bug number of #53, I think there's
been time to "burn the diskpacks" on this one ;-).  One aspect
of the problem being that finding relevant senders of #value is not so easy,
there are over a thousand senders in my image...

    -- chris


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Schwab,Wilhelm K
Blair,

>>At this time we don't have any other solution for this issue, except to
>>make it clear that ValuePresenter>>value should be avoided inside any
>>event handler for that presenter's #valueChanged event.
>
>
> The problem with that -- and the reason I suggested that ValuePresenter>>value
> was essentially useless -- is that the place that #value is sent is quite
> likely to be several layers of software away from anywhere that knows it is
> in a #valueChanged event.

It does seem like an accident waiting to happen.  Could we add another
event (#valueChanged:??) that carries either the new value, or a value
model that has predictable contents?

If the idea is sound, you might be able to create an optional patch to
D5 to accelerate a cleanup for those who would like to get a jump on it.
  I am interested, because I have an intermittent problem that this
might explain.

My suggestion is to offer an optional patch that either adds the new
event to be triggered along with #valueChanged (inefficient but does not
break anything), or actually makes the change to D5, breaking code the
relies on #valueChanged.  Unless it turns out to be horribly inefficient
or much harder than I anticipate to tweak my code to use it, I would
jump at either.


> Still, I agree that it's tricky to find a solution that wouldn't break the
> existing code you mention (though, with a bug number of #53, I think there's
> been time to "burn the diskpacks" on this one ;-).  One aspect
> of the problem being that finding relevant senders of #value is not so easy,
> there are over a thousand senders in my image...

Which might be tricking me =:0  But I suspect that the relevant existing
code is already broken, and we just don't know it.  However, if
#valueChanged: fixes the problem, and if it ends up optionally in D5.x
and as mainstream in D6, then one could always add an optional extra
trigger to D6 to keep existing code happy until it can be modified.

Have a good one,

Bill

--
Wilhelm K. Schwab, Ph.D.
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Two MVP bugs

Chris Uppal-3
In reply to this post by Chris Uppal-3
I wrote:

> I'm also starting to wonder if it, or something like it, is behind
> another bug that I've not been able to pin down yet.  One of my tools
> uses a range of dates, each of which can be nil.  The View presents the
> range as a pair of DatePresenters, each connected to a
> ValueAspectAdapter.  Under some circumstances, that I've never been able
> to reproduce, it takes 2 toggles of the checkmark before the changes
> (toggling the internal values between a real date and nil) take effect.
> Thereafter they work fine...  Puzzling, but perhaps a relative of the
> same bug.

A bit of a follow-up to that.  I've investigated further and it is not, in
fact, related to the "value" system at all.  As far as I can see it is a bug in
the windows control itself such that it does not send its change notification
(DTN_DATETIMECHANGE) when the control is first toggled off.  E.g.  Try:

    model := Date today asValue.
    dp := DatePresenter showOn: model.
    handler := [Transcript
                        display: 'Model value is ';
                        display: model value;
                        cr].
    model when: #valueChanged send: #value to: handler.

And then the first two time you click on the big checkmark, nothing will be
traced, thereafter everything will work fine.  The first time you click it,
Windows does not send DTN_DATETIMECHANGE (although Windows does "know" that the
value is now null); the second time you click, Windows does send the
notification, but Dolphin effectively ignores it because it has no reason to
suppose that anything has changed.

I couldn't find any record of a known bug to cause this, so there's still a
possibility that something's not quite right in the way that Dolphin's driving
the control, but it all looks very simple so it's hard to imagine what could go
wrong.  Just one of those things, I suppose...

    -- chris