292089 - MIP properties are not applied on repeat content

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

relevant, constraint, etc. are not shown on repeat content. I know why (events
are sent before the repeat content is created), and also think I have a
solution. The solution could depend on bug 271720 though.

Comment 1

[Allan Beaufour]

Attachment: Testcase

Testcase

Comment 2

[Allan Beaufour]

Attachment: Patch

Fixes the bug by adding a nsIModelElementPrivate::SetStates() call that is
called by the control when it has a model and a bound node, that is in Bind().

I've moved the state setting (through attributes for now, until bug 271720 is
fixed) from the control to the model. That saves us a ton of string
comparisons, as we do not need to check for the events on the control to set
the states. I've also added some more return value checking in
nsXFormsModelElement.

I've also fixed output, which had a couple of bugs as a result of bug 284469
and
bug 289534. It now uses nsXFormsControlStub::ResetBoundNode() too, so the
correct member variables are cleared and set.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 182053 [details] [diff] [review]
Patch

Looks ok (except kStateAttributes, which is temporary)

>+nsXFormsModelElement::SetSingleState(nsIDOMElement *aElement,
>+                                     PRBool         aState,
>+                                     nsXFormsEvent  aOnEvent,
>+                                     PRUint32       aAttributePos)

Please, add big XXX to all the places where you're using this hacky
attribute position thing.

>+  if (aAllStates || ns->ShouldDispatchValid()) {
>+    SetSingleState(element, ns->IsValid(), eEvent_Valid, 0);
>+  }
>+  if (aAllStates || ns->ShouldDispatchReadonly()) {
>+    SetSingleState(element, ns->IsReadonly(), eEvent_Readonly, 2);
>+  }
>+  if (aAllStates || ns->ShouldDispatchRequired()) {
>+    SetSingleState(element, ns->IsRequired(), eEvent_Required, 4);
>   }
>-  if (aInitialize || ns->ShouldDispatchValueChanged()) {
>+  if (aAllStates || ns->ShouldDispatchRelevant()) {
>+    SetSingleState(element, ns->IsRelevant(), eEvent_Enabled, 6);

Add XXX about 0, 2, 4, 6

> 
>+const nsString kStateAttributes[8] = { NS_LITERAL_STRING("valid"),
>+                                       NS_LITERAL_STRING("invalid"),
>+                                       NS_LITERAL_STRING("read-only"),
>+                                       NS_LITERAL_STRING("read-write"),
>+                                       NS_LITERAL_STRING("required"),
>+                                       NS_LITERAL_STRING("optional"),
>+                                       NS_LITERAL_STRING("enabled"),
>+                                       NS_LITERAL_STRING("disabled") };
>+

XXX here too ;)

> 
> /**
>+ * The attribute names used on the elements to reflect the pseudo class state
>+ * until bug 271720 is landed.
>+ */
>+extern const nsString kStateAttributes[8];
>+

Add XXX and @todo

Comment 4

[Allan Beaufour]

Attachment: Patch with XXXs

Comment 5

[Allan Beaufour]

Attachment: Patch that actually applies cleanly

somebody *ahem* :), checked something in

Comment 6

[Allan Beaufour]

Chcked in.