Closed Bug 310276 Opened 19 years ago Closed 19 years ago

output isn't refreshed in some cases

Categories

(Core Graveyard :: XForms, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: surkov, Assigned: doronr)

References

Details

(Keywords: fixed1.8.0.4, fixed1.8.1)

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 Element 'output' has attribute value: <xf:output value="/example/first"/> <xf:input bind="first"/> Model has two 'bind' elements like shown below: <xf:model id="model"> <xf:instance> <example xmlns=""> <first>true</first> <second>true</second> </example> </xf:instance> <xf:bind nodeset="/example/first" id="first" type="xsd:boolean"/> <xf:bind nodeset="/example/second" id="second" type="xsd:boolean"/> </xf:model> In this case when I click on checkbox then 'refresh' event isn't posted for output. Reproducible: Always
Attached file testcase (deleted) —
Mighty weird. The first time or two you click the input, the output is not refreshed. But after a few clicks it works...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to comment #2) > Mighty weird. The first time or two you click the input, the output is not > refreshed. But after a few clicks it works... Only first time. At second time refresh event is posted.
Blocks: 326372
Blocks: 326373
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All
Assignee: aaronr → doronr
allan: in the test case, when I click on the checkbox, I get 2 changed nodes when this bug happens: the 2 instance data nodes, even though only one of them is bound to. Is that a bug? I can "fix" that (the second node in the change nodes list) by removing the type attribute off the 2nd bind.
Attached patch patch (obsolete) (deleted) — Splinter Review
Doron debugged this and helped with the patch, so if it sux, its his fault :-) The problem turned out to be that in nsXFormsModelElement::Refresh we were breaking out of the 'check for dirty dependencies' for loop when we detected that we needed to rebind the control, but we were still in the for loop that was checking for changed nodes. So we'd check the next changed node and enter the for loop for dirty dependency checking and that would change the rebind flag back to false. So in the end, we wouldn't end up rebinding the output when we should have. As far as I can tell, we should jump out of both for loops once we determine that we need to rebind the control. During this debugging Doron also noticed that we could never have mBoundNode set up if output only had a value attribute. I also thought it would be more efficient not to call ProcessNodeBinding under Bind if it couldn't set us up with a bound node anyhow. So I added the parts of ProcessNodeBinding that assigns mModel and adds the control to the deferred bind list to output's Bind code. The rest of the services that ProcessNodeBinding provides can come with the call to it during output's Refresh (refresh already follows almost immediately after every call to bind)
Attachment #215944 - Flags: review?(allan)
Attachment #215944 - Flags: review?(smaug)
Comment on attachment 215944 [details] [diff] [review] patch oh, correct. change the output changes to an early return (if (!mHasBinding) ... return). With that r=me
Attachment #215944 - Flags: review?(allan) → review+
Status: NEW → ASSIGNED
Comment on attachment 215944 [details] [diff] [review] patch > >- for (PRInt32 j = 0; j < mChangedNodes.Count(); ++j) { >+ for (PRInt32 j = 0; j < mChangedNodes.Count() && !rebind; ++j) { > curChanged = do_QueryInterface(mChangedNodes[j]); > > // Check whether the bound node is dirty. If so, we need to refresh the > // control (get updated node value from the bound node) > if (!refresh && boundNode) { perhaps add a comment about why we are doing a !rebind?
Attached patch patch with nits fixed (deleted) — Splinter Review
fixes nits by Allan and Doron
Attachment #215944 - Attachment is obsolete: true
Attachment #216043 - Flags: review?(smaug)
Attachment #215944 - Flags: review?(smaug)
Attachment #216043 - Flags: review?(smaug) → review+
checked into the trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Blocks: 332853
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: