subwidgets ending up marked as dirty?

Hi all,

I am playing with Iliad and trying to make a Google Maps API.  I'm having
a problem designing the Javascript layer.  While my problem is due to
overriding some private methods on my part, I think it can be reproduced
as well in this hypothetical situation:

1) a stateful widget has setters that use #markDirty after doing their
job;

2) one of the setters is called from within the answer handler of an
ILSequence;

3) later, the same widget is shown again in the ILSequence.

After step 2, #beDirty is called on a decorated widget. After step 3,
the widget is rendered twice.  This is usually unnoticeable.


Now, however if you add:

4) the #contents method writes javascript in a way that is not idempotent

... you get two executions of this javascript.

The problem (I won't call it a bug) can happen without sequences as well,
of course, but sequences are the only case I can think of when you'll find
non-idempotent javascript.

I kind of fixed it with this patch:

diff --git a/Core/Sessions/ILStateRegistry.st b/Core/Sessions/ILStateRegistry.st
index 7bd8119..424df57 100644
--- a/Core/Sessions/ILStateRegistry.st
+++ b/Core/Sessions/ILStateRegistry.st
@@ -92,6 +92,11 @@ ILObject subclass: ILStateRegistry [
  states := nil
     ]
 
+    isWidgetDirty: aWidget [
+ <category: 'states'>
+ ^(self stateAt: aWidget) ~= aWidget state
+    ]
+
     dirtyWidgets [
  "Answer all widgets which state has changed"
  <category: 'states'>
@@ -99,8 +104,10 @@ ILObject subclass: ILStateRegistry [
  | dirtyWidgets |
  dirtyWidgets := OrderedCollection new.
  self widgets do: [:each |
-    (self stateAt: each) = each state ifFalse: [
- dirtyWidgets add: each]].
+    ((self isWidgetDirty: each) and: [
+                each owner isNil
+                    or: [ (self isWidgetDirty: each owner) not ]])
+                        ifTrue: [dirtyWidgets add: each]].
  ^dirtyWidgets
     ]
 ]

However, the patch seems very wrong to me.

Is it simply wrong to have non-idempotent javascript?  Should the answer
handlers be run still with the owner?

If the patch is correct, could #markDirty be simplified to do just #beDirty
plus the loop on the dependent widgets?

Thanks,

Paolo

Hi Paolo!

Le lundi 02 août 2010 à 02:37 +0200, Paolo Bonzini a écrit :
> Hi all,
>
> I am playing with Iliad and trying to make a Google Maps API.
Very cool!
No, not at all!

>
> If the patch is correct, could #markDirty be simplified to do just #beDirty
> plus the loop on the dependent widgets?

I have to think about about that. I remember working on the #markDirty
and adding this owner stuff, but I can't remember why... I can't see a
scenario where this is the right behaviour.

Cheers,

--
Nicolas Petton

ObjectFusion S.A.R.L.
Applications web, consulting, design
http://www.objectfusion.fr

On 08/03/2010 10:31 AM, Nicolas Petton wrote:
>> If the patch is correct, could #markDirty be simplified to do just #beDirty
>> plus the loop on the dependent widgets?
>
> I have to think about about that. I remember working on the #markDirty
> and adding this owner stuff, but I can't remember why... I can't see a
> scenario where this is the right behaviour.

Well, the owner stuff is what makes this bug very hard to trigger. :)
Without it all subwidgets would be built twice.

So maybe the above patch is incorrect _but_ the patch plus simplifying
#markDirty is not that bad.

Paolo

Le mardi 03 août 2010 à 10:38 +0200, Paolo Bonzini a écrit :

> So maybe the above patch is incorrect _but_ the patch plus simplifying
> #markDirty is not that bad.

Ahah, I didn't think about that :)

--
Nicolas Petton

ObjectFusion S.A.R.L.
Applications web, consulting, design
http://www.objectfusion.fr

Paolo, I applied your patch + simplified #markDirty with the following
changes:

ILStateRegistry>>isWidgetDirty: does the owner testing stuff.
Also, it does it recursively.
I also simplified ILStateRegistry>>dirtyWidgets.

Thanks again!
Nico

--
Nicolas Petton

ObjectFusion S.A.R.L.
Applications web, consulting, design
http://www.objectfusion.fr