329633 - We should not dispatch events on form load

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

Form construction does not include an xforms-refresh:
http://www.w3.org/TR/2005/PER-xforms-20051006/slice4.html#evt-modelConstruct
so we should not dispatch events on load.

Comment 1

[Allan Beaufour]

Attachment: Testcase

Comment 2

[Allan Beaufour]

Attachment: Work in progress

Hmmm, we might, internally, rely on the xforms-refresh for external instances, and I actually think that the mModel->SetStates() is wrong. The states should be set during the xforms-refresh cycle, not if just Bind() gets called.

Comment 3

[Allan Beaufour]

Attachment: Patch

This removes the xforms-refresh event on form construction.

I've done some minimal testing of external instances, misc. controls, and dynamically changing attributes and inserting elements. It seems to work for me.

There is still something wrong with out Reval/Refresh setup, and when the SetStates gets triggered by controls, but that should be fixed on the (long awaited) fix to our entire RRR routine, which is bug 300591.

Comment 4

[aaronr]

Comment on attachment 214411 [details] [diff] [review]
Patch

its a potentially huge change where I would expect *something* to fail, but Allan is right...everything I tested worked.  The only hole I see is things that are added to the model list that aren't bound to an instance node (and thus on the deferred bind list) may not get refreshed during load now.  Do we have anything anymore that fits this bill?

Comment 5

[Allan Beaufour]

Comment on attachment 214411 [details] [diff] [review]
Patch

(In reply to comment #4)
> (From update of attachment 214411 [details] [diff] [review] [edit])
>The only hole I see is things
> that are added to the model list that aren't bound to an instance node (and
> thus on the deferred bind list) may not get refreshed during load now.  Do we
> have anything anymore that fits this bill?

I haven't found anything, no.

Comment 6

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Comment on attachment 214411 [details] [diff] [review]
Patch

I wonder if we'll need that aAllStates at some point. But r=me

Comment 7

[Allan Beaufour]

Checked into trunk.

Comment 8

[Allan Beaufour]

This regressed on trunk

Comment 9

[Allan Beaufour]

This is probably to blame.
http://lxr.mozilla.org/seamonkey/source/extensions/xforms/nsXFormsControlStub.cpp#212

I keep getting issues over and over again because it's a bit "transparent" when we are initializing and when we do normal processing. It would be nice with a more explicit process. Repeats also suffer from this.

Comment 10

[Allan Beaufour]

Attachment: Fix

Check for mReadyHandled before sending events.

The reason for this problem occuring is that the responsibility for setting the states (and sending the events) was moved from the model to the controls, and I forgot about inhibiting the events on load.

Comment 11

[Allan Beaufour]

(In reply to comment #10)
> Created an attachment (id=219872) [edit]

Fixed on trunk.
/cvsroot/mozilla/extensions/xforms/nsXFormsModelElement.cpp,v  <--  nsXFormsModelElement.cpp
new revision: 1.113; previous revision: 1.112