Last Comment Bug 329633 - We should not dispatch events on form load
: We should not dispatch events on form load
Status: RESOLVED FIXED
: fixed1.8.0.4, fixed1.8.0.5, fixed1.8.1
Product: Core
Classification: Components
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Allan Beaufour
: Stephen Pride
Mentors:
http://www.w3.org/TR/xforms/
Depends on:
Blocks: 326556 332853
  Show dependency treegraph
 
Reported: 2006-03-07 09:02 PST by Allan Beaufour
Modified: 2006-06-06 07:00 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (1.65 KB, application/xhtml+xml)
2006-03-07 09:03 PST, Allan Beaufour
no flags Details
Work in progress (5.72 KB, patch)
2006-03-07 09:36 PST, Allan Beaufour
no flags Details | Diff | Review
Patch (5.64 KB, patch)
2006-03-08 02:02 PST, Allan Beaufour
aaronr: review+
bugs: review+
Details | Diff | Review
Fix (1.09 KB, patch)
2006-04-26 07:05 PDT, Allan Beaufour
bugs: review+
aaronr: review+
Details | Diff | Review

Description Allan Beaufour 2006-03-07 09:02:53 PST
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 2006-03-07 09:03:23 PST
Created attachment 214318 [details]
Testcase
Comment 2 Allan Beaufour 2006-03-07 09:36:27 PST
Created attachment 214320 [details] [diff] [review]
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 2006-03-08 02:02:58 PST
Created attachment 214411 [details] [diff] [review]
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 2006-03-14 13:47:40 PST
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 2006-03-15 00:22:26 PST
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] (high review load, please consider other reviewers) 2006-03-15 00:57:58 PST
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 2006-03-15 01:18:15 PST
Checked into trunk.
Comment 8 Allan Beaufour 2006-04-25 07:31:26 PDT
This regressed on trunk
Comment 9 Allan Beaufour 2006-04-25 07:35:57 PDT
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 2006-04-26 07:05:35 PDT
Created attachment 219872 [details] [diff] [review]
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 2006-04-27 01:43:24 PDT
(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

Note You need to log in before you can comment on or make changes to this bug.