Note: There are a few cases of duplicates in user autocompletion which are being worked on.

output isn't refreshed in some cases

RESOLVED FIXED

Status

Core Graveyard
XForms
P2
normal
RESOLVED FIXED
12 years ago
a year ago

People

(Reporter: surkov, Assigned: Doron Rosenberg (IBM))

Tracking

({fixed1.8.0.4, fixed1.8.1})

Trunk
fixed1.8.0.4, fixed1.8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

2.48 KB, application/xhtml+xml
Details
3.98 KB, patch
smaug
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
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
(Reporter)

Comment 1

12 years ago
Created attachment 197666 [details]
testcase

Comment 2

12 years ago
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
(Reporter)

Comment 3

12 years ago
(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.

Updated

12 years ago
Blocks: 326372

Updated

12 years ago
Blocks: 326373

Updated

12 years ago
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All

Updated

12 years ago
Assignee: aaronr → doronr
(Assignee)

Comment 4

12 years ago
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.

Comment 5

12 years ago
Created attachment 215944 [details] [diff] [review]
patch

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)

Updated

12 years ago
Attachment #215944 - Flags: review?(smaug)

Comment 6

12 years ago
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+

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 7

12 years ago
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?

Comment 8

12 years ago
Created attachment 216043 [details] [diff] [review]
patch with nits fixed

fixes nits by Allan and Doron
Attachment #215944 - Attachment is obsolete: true
Attachment #216043 - Flags: review?(smaug)
Attachment #215944 - Flags: review?(smaug)

Updated

12 years ago
Attachment #216043 - Flags: review?(smaug) → review+

Comment 9

12 years ago
checked into the trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

12 years ago
Blocks: 332853

Updated

12 years ago
Keywords: fixed1.8.0.3, fixed1.8.1

Updated

12 years ago
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.