278211 - Implement <insert> and <delete>

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

Comment 1

[aaronr]

When hooking this up, please generate the xforms-insert and xforms-delete events
to the xforms:instance element if you think about it.

Comment 2

[Allan Beaufour]

Attachment: Patch

It does not support repeat-index, as it 1) depends on bug 278209 and 2) will be
a pain to implement. New bug for that.

It also needs to set the context info for the events, but that's what bug
280423 is for.

Comment 3

[Allan Beaufour]

Attachment: Showcase

Comment 4

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

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

>+  nsCOMPtr<nsIDOMXPathResult> nodeset;
>+  rv = nsXFormsUtils::EvaluateNodeBinding(mElement,
>+                                          0,
>+                                          NS_LITERAL_STRING("nodeset"),
>+                                          EmptyString(),
>+                                          nsIDOMXPathResult::ORDERED_NODE_SNAPSHOT_TYPE,
>+
>+                                          getter_AddRefs(model),
Remove that extra newline.


>+    /// @bug This is not correct, the spec says that it should be treated like
>+    /// 'round(@at)' (XXX)
So why not implement this now?


>+  nsCOMPtr<nsIDOMNode> newNode;
>+  prototype->CloneNode(PR_TRUE, getter_AddRefs(newNode));
>+  NS_ENSURE_STATE(newNode);
No need to clone if mIsInsert==PR_FALSE


>+  // Dispatch refreshing events to the model
>+  if (aParentAction) {
>+    aParentAction->SetRecalculate(modelElem, PR_TRUE);
>+    aParentAction->SetRevalidate(modelElem, PR_TRUE);
>+    aParentAction->SetRefresh(modelElem, PR_TRUE);
Set rebuild flag too:
see http://www.w3.org/TR/xforms/slice10.html#action-action


>+    nsXFormsUtils::DispatchEvent(modelElem, eEvent_Recalculate);
>+    nsXFormsUtils::DispatchEvent(modelElem, eEvent_Revalidate);
>+    nsXFormsUtils::DispatchEvent(modelElem, eEvent_Refresh);
Dispatch rebuild too (?)

With those r=me

Comment 5

[Allan Beaufour]

(In reply to comment #4)
> (From update of attachment 172896 [details] [diff] [review] [edit])
> >+    /// @bug This is not correct, the spec says that it should be treated like
> >+    /// 'round(@at)' (XXX)
> So why not implement this now?

Mighty good question Mr. Pettay :) The answer: I forgot... I wanted to ask David
whether <setindex> should round() its @index too. That's unknown for now. I've
implemented it for insert/delete now.

Comment 6

[Allan Beaufour]

Attachment: Fixed smaug's comments

Since I'm now also dispatching xforms-rebuild, I've had to comment out
mMDG.Rebuild() in the model, since it's non-functioning until bug 279957 is
landed.

Comment 7

[Allan Beaufour]

Attachment: Updated to current CVS

... and fixed yet another slip wrt 279957

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 173066 [details] [diff] [review]
Updated to current CVS

>--- xforms/nsXFormsInsertDeleteElement.cpp	1970-01-01 01:00:00.000000000 +0100
>+++ xforms.insertdelete/nsXFormsInsertDeleteElement.cpp	2005-02-01 13:48:51.871606160 +0100
>+ * Portions created by the Initial Developer are Copyright (C) 2004

2005

>+/**
>+ * Implementation of the XForms \<insert\> and \<delete\> elements.
>+ *
>+ * @see http://www.w3.org/TR/xforms/slice9.html#action-insert
>+ *
>+ * @todo The spec. says that the events must their Context Info to:

There seems to be a word missing between "must" and "their".

>+ * "Path expression used for insert/delete (xsd:string)" (XXX)
>+ * @see http://www.w3.org/TR/xforms/slice4.html#evt-insert
>+ * @see https://bugzilla.mozilla.org/show_bug.cgi?id=280423
>+ *
>+ * @todo Any \<repeat\> elements needs to set their repeat-indexes properly if

s/needs/need/

>+    // Make casting act as round()
>+    atDoub += 0.5;
>+    /// @todo Isn't there a better (Mozilla) way of doing this conversion?
>+    /// (XXX)
>+    atInt = (PRUint32) atDoub;

You could do floor(atDoub + 0.5), we use that construct in other places.

>+  }
>+  if (!atInt || atInt > setSize)
>+    atInt = setSize;
>+  
>+
>+  //
>+  // 3) Get @position (if \<insert\>)
>+  nsString position;

nsAutoString

Looks ok otherwise.

Comment 9

[Allan Beaufour]

Fixed Brian's comments and checked in.