280823 - MIP "readonly=true()" with "calculate" doesn't display calculated value in bound field

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Stephen Pride]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050201 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050201 Firefox/1.0+

According to "6.1.2 The readonly Property" of the spec:

"Default Value: false(), unless a calculate property is specified, then true()."

However, if a "readonly=true()" property is given with a calculate property, the
field does not display/update the calculated value for bound input fields.  I
tested this with formsPlayer, and although it allows input (which it shouldn't),
it reverts to the calculated value when focus is lost.  With Mozilla (and
Novell), the input field does not allow change (which it shouldn't), but it also
doesn't display/update the calculated the value.

I'll add a test case for more clarity.

Reproducible: Always

Comment 1

[Stephen Pride]

Attachment: Test case that shows the problem

Enter a valid numeric value for "Value 1" and/or "Value 2", and tab out of the
associated input field.  The value in "Total" should update the sum of the
values in both "Value 1" and "Value 2" but doesn't.

Comment 2

[Allan Beaufour]

readonly = readonly, having reading problems? :-) Yeah, sorry for that. There
was some discussion in the WG about this, never got around to fix it.

Comment 3

[Allan Beaufour]

Attachment: Fix

Adds a parameter to SetNodeValue that tells whether it's a @calculate attribute
that tries to change the node value. The call is shielded to the "public".

Comment 4

[aaronr]

I don't want to r- because maybe I'm missing something.  But why not just add a
parameter to SetNodeValue that defaults to PR_FALSE instead of introducing a
SetNodeValueInternal?  Or do you want to also eliminate the ability of someone
setting the marknode parameter except for the engine and just doing it all in
one fix?

Comment 5

[Allan Beaufour]

(In reply to comment #4)
> I don't want to r- because maybe I'm missing something.  But why not just add a
> parameter to SetNodeValue that defaults to PR_FALSE instead of introducing a
> SetNodeValueInternal?  Or do you want to also eliminate the ability of someone
> setting the marknode parameter except for the engine and just doing it all in
> one fix?

Second one, there was no reason for the marknode parameter to be exposed to the
public. It's a matter of keeping things simple to the public.

Comment 6

[aaronr]

Comment on attachment 173740 [details] [diff] [review]
Fix

sounds good to me.

Comment 7

[Allan Beaufour]

Could you take a quick look at it too smaug?

Comment 8

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Comment on attachment 173740 [details] [diff] [review]
Fix

>+   */
>+  nsresult SetNodeValueInternal(nsIDOMNode      *aContextNode,
>+                                const nsAString &aNodeValue,
>+                                PRBool           aMarkNode = PR_TRUE,
>+                                PRBool          *aNodeChanged = nsnull,
>+                                PRBool           aIsCalculate = PR_FALSE);

Looks good, but maybe the aNodeChanged could be after aIsCalculate.
I'd like to keep the |out| parameters in the end of the parameter list.
I didn't test the patch but r=me too.

Comment 9

[Allan Beaufour]

Rearranged arguments as smaug suggested and checked in.