Last Comment Bug 393416 - Insert or delete of one node can trigger xforms-value-changed on a separate form control
: Insert or delete of one node can trigger xforms-value-changed on a separate f...
: fixed1.8.1.12
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: unspecified
: x86 Windows XP
-- normal with 1 vote (vote)
: ---
Assigned To: John L. Clark
Depends on:
Blocks: 410239
  Show dependency treegraph
Reported: 2007-08-23 08:14 PDT by John L. Clark
Modified: 2016-07-15 14:46 PDT (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Test form demonstrating the bug (2.07 KB, application/xhtml+xml)
2007-08-23 08:23 PDT, John L. Clark
no flags Details
Fix for this bug (2.23 KB, patch)
2007-10-10 07:08 PDT, John L. Clark
aaronr: review+
bugs: review+
Details | Diff | Splinter Review

Description User image John L. Clark 2007-08-23 08:14:44 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070725 Firefox/
Build Identifier: 2007-08-09-03-mozilla1.8

In some cases, performing an `xf:insert` or an `xf:delete` of one node can cause a form control bound to a separate node to observe an `xforms-value-changed` event.  This can inadvertently trigger events bound to that event even when the corresponding bound node hasn't actually changed.  I will attach a test case that illustrates this issue.

Reproducible: Always
Comment 1 User image John L. Clark 2007-08-23 08:23:05 PDT
Created attachment 277909 [details]
Test form demonstrating the bug

First, select a value for "Field 1"; a message will appear saying "xforms-value-changed observed at `field1`".  This is expected (as the value for the bound node has changed).  Now activate the "Delete" trigger, and the same message will appear.  Finally, activate the "New" trigger, and the message will appear again.  These last two messages should not appear (or to be more precise, the `xforms-value-changed` event that led to these messages should not happen), because the value of the `field1` element has not been changed.

There are several interesting things to note about this form.  If you remove the `xf:bind` in the form (lines 31-32), the "extra" `xforms-value-changed` events are no longer fired.  If you uncomment the `xf:rebuild` and `xf:refresh` actions at lines 53 and 63, respectively, then the corresponding `xf:delete` and `xf:insert` actions no longer cause the "extra" `xforms-value-changed` events to be fired.  Finally, if you only uncomment the `xf:refresh` from those pairs, the triggers need to be pressed twice before the corresponding `DOMActivate` action happens.
Comment 2 User image John L. Clark 2007-10-10 07:08:49 PDT
Created attachment 284300 [details] [diff] [review]
Fix for this bug

I believe that this patch safely fixes the first problem mentioned in this bug report.  The last chunk of the patch is the only one of real interest; the other chunks simply tweak or clean up some debug messages.  The last chunk moves the setting of the `eFlag_DISPATCH_VALUE_CHANGED` flag into the body of the `g->HasExpr()` test.  As a result, an "xforms-value-changed" should only be fired for a dirty calculate node that also has an expression.  Now, a calculate node without an expression is a start node for the dependency graph, so it will not undergo a change itself, and so should not indicate that its value changed.

Upon closer reading of the spec, I don't think the other issue is a bug; manually triggering a refresh using the `xf:refresh` does clear the refresh flag, so even though an insert or a delete was performed, only the rebuild, recalculate, and revalidate sequence will happen at the end of the outer action, so the display will not be updated until the *next* time a refresh is triggered.
Comment 3 User image alexander :surkov 2007-10-10 07:13:09 PDT
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

John you need to ask someone for review, for example, Aaron :)
Comment 4 User image aaronr 2007-11-06 09:38:50 PST
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

makes sense to me.  r=me
Comment 5 User image Olli Pettay [:smaug] 2007-11-06 12:16:14 PST
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

(Usually when making patches, -u8 -p are good parameters for cvs diff.)
Comment 6 User image aaronr 2007-11-12 15:41:20 PST
checked into trunk for jlc
Comment 7 User image aaronr 2008-01-08 19:24:38 PST
checked into 1.8 branch via bug 410239.

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