ok... it is more complicated than I thought... this is what I dug for now (sorry, this is
large):
1) the fix proposed originaly is still necesary.
2) GLMBrowser>>unregisterFromAllAnnouncements needs to be defined this way:
unregisterFromAllAnnouncements
super unregisterFromAllAnnouncements.
self panes do: [:each |
each presentations unregisterFromAllAnnouncements ].
self transmissions do: [ :each |
each destination pane unregisterFromAllAnnouncements ]
(this include panes in transmissions in the clean up)
3) this is the ugly part: there is an identity problem, because GLMUpdateActions are being
copied a lot of times, to ensure the browser function. And unsubscribe: checks for
identical objects (==) to remove from the annoncements. So... and THIS IS NOT THE
SOLUTION, just a validation of the problem. I did:
a) I added a method to Announcer:
Announcer>>unsubscribeNonIdentical: anObject
subscriptions ifNil: [ ^ self ].
subscriptions keysAndValuesDo: [ :class :actions |
subscriptions at: class put: (actions
reject: [ :each | each receiver = anObject ]) ].
subscriptions keysAndValuesRemove: [ :class :actions |
actions isEmpty ]
(it just changes the check by == to a check by =)
b) assign an unique id, = and hash to GLMUpdateAction:
id
^id ifNil: [ id = UUID new asString ]
= other
self species = other species ifFalse: [ ^false ].
^self id = other id
hash
^self species hash bitXor: self id hash
c) modify
GLMUpdateAction>>unregisterFromAllAnnouncements
self announcerObjects
do: [:each | each unsubscribeNonIdentical: self ]
and with all this changes... announcement un-suscription is working :P
this is the test who validates it:
GLMUpdateAction>>testUnregisterAnnouncementsWhenUpdatingPane
| announcer browser presentation |
announcer := Announcer new.
browser := GLMTabulator new.
browser row: #one.
browser transmit
to: #one;
andShow: [ :presenter |
(presentation := presenter list)
updateOn: TestAnnouncement from: [ announcer ];
display: [ :start | start to: 10 ] ].
browser startOn: 1.
presentation registerAnnouncements.
browser unregisterFromAllAnnouncements.
self assert: (announcer instVarNamed: 'subscriptions') isEmpty.
So... this makes the un-suscription works, but the whole "id" thing is
really-really ugly. So, as I don't know much of Glamour, maybe you can say me a real
fix for this :)
Cheers,
Esteban
pd: I attached all this as a comment on issue
http://code.google.com/p/moose-technology/issues/detail?id=492
El 15/01/2011, a las 10:43a.m., Tudor Girba escribió:
I forgot to mention that right now we have two tests, but they do not
capture the problem:
GLMBrowserTest>>testUnregisterAnnouncements
GLMBrowserTest>>testUnregisterAnnouncementsWhenRemovingPane
Cheers,
Doru
On 15 Jan 2011, at 14:41, Tudor Girba wrote:
> Excellent catch, Esteban!
>
> I know about this problem since half a year, but I could not figure out where the
problem was.
>
> I opened a ticket:
>
http://code.google.com/p/moose-technology/issues/detail?id=492
>
> Before integrating the fix, could you provide a test case that fails with the current
implementation?
>
> Cheers,
> Doru
>
>
> On 15 Jan 2011, at 14:29, Esteban Lorenzano wrote:
>
>> Hi,
>> I noticed that in some complex updating between panels, using an external
announcer, some of the update subscriptions were not removed when the browser is closed. I
debugged a little and I founded that this implementation:
>>
>> GLMUpdateAction>>unregisterFromAllAnnouncements
>> announcerObjects ifNotNil: [
>> announcerObjects do: [:each |
>> each unsubscribe: self ] ]
>>
>> is bugged, because if announcerObjects are not previously computed (and in some
cases that's what happens), the subscription is not removed.
>> This implementation (just using the accessor instead the direct object), solves
the problem (but I don't know is it's a right fix, and it should be a fix in other
place)
>>
>> GLMUpdateAction>>unregisterFromAllAnnouncements
>> self announcerObjects ifNotNil: [ :objects |
>> objects do: [:each |
>> each unsubscribe: self ] ]
>>
>> Cheers,
>> Esteban
>> _______________________________________________
>> Moose-dev mailing list
>> Moose-dev(a)iam.unibe.ch
>>
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
> --
>
www.tudorgirba.com
>
> "In a world where everything is moving ever faster,
> one might have better chances to win by moving slower."
>
>
>
--
www.tudorgirba.com
"There are no old things, there are only old ways of looking at them."
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev