Insert or delete of one node can trigger xforms-value-changed on a separate form control

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
10 years ago
a year ago

People

(Reporter: John L. Clark, Assigned: John L. Clark)

Tracking

({fixed1.8.1.12})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
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
(Assignee)

Comment 1

10 years ago
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.
(Assignee)

Comment 2

10 years ago
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

10 years ago
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

John you need to ask someone for review, for example, Aaron :)
Attachment #284300 - Flags: review?(aaronr)

Comment 4

10 years ago
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

makes sense to me.  r=me
Attachment #284300 - Flags: review?(aaronr)
Attachment #284300 - Flags: review?(Olli.Pettay)
Attachment #284300 - Flags: review+

Comment 5

10 years ago
Comment on attachment 284300 [details] [diff] [review]
Fix for this bug

(Usually when making patches, -u8 -p are good parameters for cvs diff.)
Attachment #284300 - Flags: review?(Olli.Pettay) → review+

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

10 years ago
Assignee: nobody → jlc6

Comment 6

10 years ago
checked into trunk for jlc
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

10 years ago
Blocks: 410239

Comment 7

10 years ago
checked into 1.8 branch via bug 410239.
Keywords: fixed1.8.1.12
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.