289434 - xbl-ized widgets for xforms

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Andrew Douglas]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050305
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050305

We should use xbl for defining the visual content of the xforms widgets and
leave the xtf elements to just define the functionality (such as bindings,
etc.). See http://lab.cph.novell.com/~abeaufour/xforms/uidesign.html

Reproducible: Always

Comment 1

[Andrew Douglas]

Attachment: initial patch for input, output and trigger

Comment 2

[Andrew Douglas]

Attachment: trigger test case.

a bit of a hacked test case.. for some reason getElementById was causing me
problems.. not sure if it was just because it was late or if we have an actual
issue.

Comment 3

[Andrew Douglas]

Attachment: second rough draft

still just input, output and trigger. This is mostly cleanup, fixing where the
labels show up in the xbl contents, output value works (right now it's an ugly
refresh that should probably go into the delegate.. but I don't know where),
and the first attempt at getting context working. It works AFTER the first
refresh on the page.. so it's probably something is just not set up when it
first attempts to getcontext. Note: repeats still have issues so make sure to
test outside of a <repeat>
Suggestions for improvements welcome. I hope to get label xbl-ized next and
then deal with repeat.

Other issues to be wary of: ID and class have issues... also
psuedo-elements/classes. I'm sure there are a few others.

Comment 4

[Allan Beaufour]

Attachment: updated to current CVS

Comment 5

[Andrew Douglas]

Attachment: update based on XTFBindable

This is based on the patch for XTFBindableElement in bug 291026. This fixes
context issues, repeat issues, class/id issues and generally just works. Label
is also now xbl-ized. This is pretty much the final form but just requires some
cleanup and then I'll ask for review. If there are any major gotcha's, please
let me know.
-Andrew

Comment 6

[Andrew Douglas]

Attachment: updated test case.

We do apparently have an issue with user-defined xbl bindings. The
appearance="beaufour" isn't using the css defined -moz-binding. This may be an
issue with the XTFBindableElement patch.

Comment 7

[Andrew Douglas]

Attachment: removed some useless files from the patch

sorry for all the spam

Comment 8

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

Attachment: Few fixes, cleaning up things

Comment 9

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

Attachment: updated test case, using "inline" xbl

Comment 10

[Allan Beaufour]

(In reply to comment #8)
> Created an attachment (id=181662) [edit]
> Few fixes, cleaning up things

Why change all the return values from NS_IMETHODIMP to nsresult in
nsXFormsControlStubBase? Aren't they interface implementations?

Comment 11

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

(In reply to comment #10)
> (In reply to comment #8)
> > Created an attachment (id=181662) [edit] [edit]
> > Few fixes, cleaning up things
> 
> Why change all the return values from NS_IMETHODIMP to nsresult in
> nsXFormsControlStubBase? Aren't they interface implementations?

nsXFormsControlStubBase does not inherit nsXFormsXMLVisualStub.
So the methods, which return nsresult aren't actually interface
implementations. And some methods do return NS_IMETHODIMP, those which
are from nsIXFormsControl.

Comment 12

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

Attachment: Label should work now inside repeats

This patch has a hack to delay the refreshing of the XBL widgets when they
are attached to the elements. Also try-catch is needed in many places in XBL
to ensure that we don't see unnecessary exceptions.

Comment 13

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

Attachment: <output> inside <repeat> or <message> works.

Comment 14

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

Attachment: XBL widgets initialized properly (without using a timer)

This might be almost the final patch. Now just trying to get the reviews for
Bug 291026

Comment 15

[aaronr]

I applied the patch for 291026 on a build from 05/09 and then this patch on top
of it.  It won't build on Windows due to the call to SetClassAttributeName in
::Created  in nsXFormsControlStub.cpp on the aWrapper.  SetClassAttributeName
isn't a method on a nsIXTFElementWrapper, but rather a
nsIXTFStyledElementWrapper.  Shouldn't there be a QI before the call?  I also
had a problem with nsXFormsOutputElement::Bind calling
nsXFormsControlStub::Bind() directly since it now inherits from
nsXFormsControlStubBase.

Could be I screwed up when fixing up the diff, too. :=)  But could someone
please see if my errors were just my machine before this patch gets checked in?
 Thanks.

Comment 16

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

(In reply to comment #15)
> I applied the patch for 291026 on a build from 05/09 and then this patch on top
> of it.  
> ...
> Could be I screwed up when fixing up the diff, too. :=)  But could someone
> please see if my errors were just my machine before this patch gets checked in?
>  Thanks.

Sorry, this patch is not updated to work with the latest patch in bug 291026.

Comment 17

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

Attachment: works with the latest patch in bug 291026

testcase:
http://www.cs.helsinki.fi/u/pettay/moztests/xforms/dynamic_ui/calc_svg.xhtml

Comment 18

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

Comment on attachment 183423 [details] [diff] [review]
works with the latest patch in bug 291026

Time to review? XTFB got the sr+

Comment 19

[aaronr]

Here is the functionality that we currently have with XTF that we don't have
with this XBL patch (as far as I can tell, running through most of my
testcases).  Should we wait until they are addressed, or fix them all as
seperate bugs after this patch is in?

1) readonly='true()' MIP doesn't get reflected when specified on bindings to
inputs.  Input values can be changed.
2) no submit element
3) clicking/unclicking checkbox doesn't change bound data
4) removing binding attributes via script doesn't work well.
5) output elements contained inside repeat elements don't work (using the repeat
as the context for their value).
6) If I have an ephemeral message that contains a xhtml:input as a child, that
xhtml:input is not focusable.
7) setfocus element doesn't work
8) minimal trigger doesn't fire domactivates when clicked.
9) input with incremental="true" doesn't work.

Comment 20

[Allan Beaufour]

(In reply to comment #19)
> Here is the functionality that we currently have with XTF that we don't have
> with this XBL patch (as far as I can tell, running through most of my
> testcases).  Should we wait until they are addressed, or fix them all as
> seperate bugs after this patch is in?

Thanks for testing it Aaron. I haven't look at the patch yet, but I think we
should try fixing your comments now. If some of them are really troublesome,
then yes, then let's create new bugs. But let's see if we cannot fix most of
them now

Comment 21

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

(In reply to comment #20)
> (In reply to comment #19)
> > Here is the functionality that we currently have with XTF that we don't have
> > with this XBL patch (as far as I can tell, running through most of my
> > testcases).  Should we wait until they are addressed, or fix them all as
> > seperate bugs after this patch is in?
> 
> Thanks for testing it Aaron. I haven't look at the patch yet, but I think we
> should try fixing your comments now. If some of them are really troublesome,
> then yes, then let's create new bugs. But let's see if we cannot fix most of
> them now

hmm, 5. was working, I think, apparently I have to re-test.
6. is weird test case ;)

I'll update the patch soon(ish).

thanks for testing Aaron.

Comment 22

[aaronr]

Ok, since it looks like you'll try to fix it before checkin, better point you to
some testcase :=)  Didn't want to clog up the attachment list.  Most live in
bugzilla already, anyhow.

1) readonly input problem -
https://bugzilla.mozilla.org/attachment.cgi?id=173199.  Shouldn't be able to
edit the 'total' field.
2) submit button not implemented -
http://xformstest.org/mozilla/2005/02/03/test-get-1.xhtml.  Should see a
'submit' control following the input controls.
3) checkbox not updating bound data -
https://bugzilla.mozilla.org/attachment.cgi?id=174637  Checking and unchecking
the checkbox should change the value next to the submit control.
4) manipulating binding attrs via DOM -
https://bugzilla.mozilla.org/attachment.cgi?id=178989  Tough to explain in a
couple of sentences.  Use latest nightly next to a XBL patched build, you'll see
the differences in clicking the buttons.
5) repeat with outputs problem -
https://bugzilla.mozilla.org/attachment.cgi?id=168730  Should see the numbers
10, 30 and 45
6) ephemeral containing html:input - will attach testcase
7) set focus problem - I will attach testcase
8) minimal trigger - user error.  Seems to work when I try it now.  However,
won't generate a DOMActivate when clicking with the right mouse button with XBL
patch, but does with XTF.  Which is behavior is right?  testcase:
https://bugzilla.mozilla.org/attachment.cgi?id=175205
9) incremental input - https://bugzilla.mozilla.org/attachment.cgi?id=171769

Comment 23

[aaronr]

Attachment: setfocus testcase

testcase for setfocus.	Click on the trigger, input focus should move to the
input control

Comment 24

[aaronr]

Attachment: testcase for ephemeral message

testcase for ephemeral message containing a html:input.  Click on the trigger,
you'll see some html appear.  click on the input field with the left mouse. 
Under XTF, this input will get input focus.  Under XBL, message will disappear.

Comment 25

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

I've fixed other test cases except 6, which is quite strange one and not
working well with XTF controls either.
In 8 the old behaviour is wrong.

Patch coming.

Comment 26

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

Attachment: for review

Allan, maybe you could review this one too ;)

Comment 27

[Allan Beaufour]

(In reply to comment #17)
> Created an attachment (id=183423) [edit]
> works with the latest patch in bug 291026
> 
> testcase:
> http://www.cs.helsinki.fi/u/pettay/moztests/xforms/dynamic_ui/calc_svg.xhtml

Try this WITHOUT this patch, and switch between Normal and SVG twice... my build
crashes in layout/style/nsStyleContext.cpp... can you look at that Olli?

Comment 28

[Allan Beaufour]

Attachment: Testcase: Output with invalid binding

This one segfaults Mozilla...

Comment 29

[Allan Beaufour]

Attachment: Testcase: Dynamic insertion of output

This does not work with the patch. Feel free to ignore it for now, and create a
new bug though (just remember it :) ).

Comment 30

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

Attachment: for review 2

The crash was (again) from the nsXFormsUtils::ReportError.
I changed the serialization so that we don't clone nodes anymore.
I think that has been a problem earlier too.

The dynamic addition of an <output> is not yet fixed :(

Comment 31

[Allan Beaufour]

Attachment: Testcase: Listening for xforms-ready

This fails. I listen for the xforms-ready event, which should only be fired
when all controls are initialized. So I would expect to be able to get the
correct value?

Comment 32

[Allan Beaufour]

The output with "Node: 2" is not initialized correctly in attachment 181965 [details].

Comment 33

[Allan Beaufour]

(In reply to comment #27)
> (In reply to comment #17)
> > Created an attachment (id=183423) [edit] [edit]
> > works with the latest patch in bug 291026
> > 
> > testcase:
> > http://www.cs.helsinki.fi/u/pettay/moztests/xforms/dynamic_ui/calc_svg.xhtml
> 
> Try this WITHOUT this patch, and switch between Normal and SVG twice... my build
> crashes in layout/style/nsStyleContext.cpp... can you look at that Olli?

Created bug 296927 for that.

Comment 34

[Allan Beaufour]

Comment on attachment 185495 [details] [diff] [review]
for review 2

Besides comment 32, it looks pretty good.

Create new bugs for the dynamic and xforms-ready issues, and create an up to
patch and I'll do code review.

Comment 35

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

Attachment: for review 3

up-to-date and comment #32 should be fixed.

Comment 36

[Allan Beaufour]

Comment on attachment 185798 [details] [diff] [review]
for review 3

> Index: nsXFormsLabelElement.cpp
> ===================================================================

>  /**
>   * Implementation of the XForms \<label\> element.
>   * This constructs an anonymous \<span\> to hold the inline content.
>   */

Fix comment.

> Index: nsXFormsModelElement.cpp
> ===================================================================

> +nsPostRefresh::~nsPostRefresh()
> +{
> +#ifdef DEBUG_smaug
> +  printf("~nsPostRefresh\n");
> +#endif
> +  --sRefreshing;
> +  if (sPostRefreshList && !sRefreshing) {
> +    while (sPostRefreshList->Count()) {
> +      PRInt32 last = sPostRefreshList->Count() - 1;
> +      nsIXFormsControl* control =
> +        NS_STATIC_CAST(nsIXFormsControl*, sPostRefreshList->ElementAt(last));
> +      sPostRefreshList->RemoveElementAt(last);
> +      if (control)
> +        control->Refresh();
> +    }

The reason why you keep refreshing the last element is that refresh can lead
to additions/deletions in sPostRefreshList, right? Add a note about it.

> +nsXFormsModelElement::NeedsPostRefresh(nsIXFormsControl* aControl)
> +{
> +  if (sRefreshing) {
> +    if (!sPostRefreshList) {
> +      sPostRefreshList = new nsVoidArray();
> +      if (!sPostRefreshList)
> +        return;

A warning please.

> Index: nsXFormsModelElement.h
> ===================================================================

> +/**
> + * nsPostRefresh is needed by the UI Controls, which are implemented in
> + * XBL and used inside \<repeat\>.
> + */

Yes, but what is the purpose of it?

> Index: nsXFormsOutputElement.cpp
> ===================================================================
>  nsXFormsOutputElement::Bind()
>  {
> -  if (!mValue || !mHasParent)
> -    return NS_OK;
> +  nsXFormsDelegateStub::Bind();
> +
> +  mBoundNode = nsnull;
> 
>    nsresult rv;
>    rv = mElement->HasAttribute(NS_LITERAL_STRING("ref"), &mHasBinding);
>    NS_ENSURE_SUCCESS(rv, rv);
>    if (!mHasBinding) {
>      rv = mElement->HasAttribute(NS_LITERAL_STRING("bind"), &mHasBinding);
>      NS_ENSURE_SUCCESS(rv, rv);
>    }
> 
> +    nsCOMPtr<nsIDOMXPathResult> result;
>    if (mHasBinding) {
> -    rv = nsXFormsControlStub::Bind();
> +    rv = ProcessNodeBinding(NS_LITERAL_STRING("ref"),
> +                            nsIDOMXPathResult::FIRST_ORDERED_NODE_TYPE,
> +                            getter_AddRefs(result));
>    } else {
> -    mBoundNode = nsnull;
> -    // This call does a bit too much, as we do the call again in Refresh(),
> -    // but we need to clear index listeners, bind to the model, get
> -    // evt. context, etc. in Bind().
> -    rv = ResetBoundNode(NS_LITERAL_STRING("value"),
> -                        nsIDOMXPathResult::STRING_TYPE);
> -  }  
> +    rv = ProcessNodeBinding(NS_LITERAL_STRING("value"),
> +                            nsIDOMXPathResult::STRING_TYPE,
> +                            getter_AddRefs(result));
> +  }
> +  
>    NS_ENSURE_SUCCESS(rv, rv);

Why all the changes here? The first ::Bind() does a lot of work that is
canceled by setting mBoundNode = nsnull, and then redone when mHasBinding is
true. What is the problem with the original approach?

> -  return rv;
> +  if (result) {
> +    result->GetSingleNodeValue(getter_AddRefs(mBoundNode));
> +  }
> +
> +  return NS_OK;
>  }

> Index: nsXFormsTriggerElement.cpp
> ===================================================================

> -NS_NewXFormsTriggerElement(nsIXTFElement **aResult)
> -{
> -  *aResult = new nsXFormsTriggerElement();
> -  if (!*aResult)
> -    return NS_ERROR_OUT_OF_MEMORY;
> +  nsCOMPtr<nsIXFormsUIWidget> widget(do_QueryInterface(mElement));
> +  if (widget) {
> +    widget->Disable(aDisable);
> +  }

Do we really need a special function for that. Can we not just set an
attribute as HTML controls do?

> Index: nsXFormsUtils.h
> ===================================================================

> +#define NS_NAMESPACE_MOZ_XFORMS_TYPE     "http://www.mozilla.org/projects/xforms/type"

Should we include a year?

> +++ nsIXFormsDelegate.idl	2005-06-09 17:37:48.000000000 +0300

> + * Portions created by the Initial Developer are Copyright (C) 2004
> + * the Initial Developer. All Rights Reserved.
2005

> +interface nsIXFormsDelegate : nsISupports
> +{
> +  attribute DOMString value;
> +
> +  readonly attribute boolean isReadonly;
> +  readonly attribute boolean isEnabled;
> +  readonly attribute boolean isRequired;
> +  readonly attribute boolean isValid;
> +  readonly attribute boolean hasBoundNode;
> +  void widgetAttached();
> +};

Add documentation for each attribute and the function.

> +++ nsIXFormsUIWidget.idl	2005-06-09 17:37:48.000000000 +0300
> + * Portions created by the Initial Developer are Copyright (C) 2004
> + * the Initial Developer. All Rights Reserved.

2005

> +interface nsIXFormsUIWidget : nsIDOMElement
> +{
> +  void refresh();
> +  boolean focus();
> +  void attributeChanged(in AString name, in boolean removed);
> +  void disable(in boolean disable); // for <submit>
> +};

Again, documentation for each function.

> +++ nsXFormsDelegateStub.h	2005-06-09 17:37:48.000000000 +0300
> + * Portions created by the Initial Developer are Copyright (C) 2004
> + * the Initial Developer. All Rights Reserved.

2005

> +enum nsRepeatState {
> +  eType_Unknown,
> +  eType_Template,
> +  eType_GeneratedContent,
> +  eType_NotApplicable
> +};

Documentation.

And shouldn't this handling be in a class "closer" to nsIXFormsControl
instead? It should be valid for all controls.

> +protected:
> +  nsresult GetState(const nsAString &aState, PRBool *aStateVal);
> +  nsRepeatState UpdateRepeatState();
> +  nsString      mControlType;
> +  nsRepeatState mRepeatState;
> +};

Documentation.

> +++ nsXFormsDelegateStub.cpp	2005-06-09 17:37:48.000000000 +0300
> + * Portions created by the Initial Developer are Copyright (C) 2004
> + * the Initial Developer. All Rights Reserved.

2005

> +nsXFormsDelegateStub::AttributeSet(nsIAtom *aName, const nsAString &aNewValue)
> +{
> +  nsXFormsBindableControlStub::AttributeSet(aName, aNewValue);
> +
> +  nsCOMPtr<nsIXFormsUIWidget> widget(do_QueryInterface(mElement));
> +  if (widget && aName) {
> +    nsAutoString name;
> +    aName->ToString(name);
> +    widget->AttributeChanged(name, PR_FALSE);
> +  }

Why? Can this not be solved with inherited attributes in XBL? If not, include
a comment about it, here or in the interface. Interface I think.

In fact, you are not using it are you?

> +nsXFormsDelegateStub::Refresh()
> +{
> +  if (mRepeatState == eType_Template)
> +    return NS_OK;
> +
> +  NS_NAMED_LITERAL_STRING(mozTypeNs, NS_NAMESPACE_MOZ_XFORMS_TYPE);
> +  NS_NAMED_LITERAL_STRING(mozType, "type");
> +
> +  if (mModel && mBoundNode) {
> +    nsAutoString type, ns;
> +    nsresult rv = mModel->GetTypeAndNSFromNode(mBoundNode, type, ns);
> +    NS_ENSURE_SUCCESS(rv, rv);
> +
> +    ns.AppendLiteral("#");
> +    ns.Append(type);
> +    mElement->SetAttributeNS(mozTypeNs, mozType, ns);
> +  } else {
> +    mElement->RemoveAttributeNS(mozTypeNs, mozType);
> +  }

Extract the type code, so it can be used by nsXFormsOutput too, instead of the
duplicated code.

> +nsXFormsDelegateStub::TryFocus(PRBool* aOK)
> +{
> +  nsresult rv = NS_OK;
> +  if (GetRelevantState()) {
> +    nsCOMPtr<nsIXFormsUIWidget> widget = do_QueryInterface(mElement);
> +    NS_ENSURE_STATE(widget);

Wouldn't it be ok to just return false instead of failing if there's no widget?

> +nsXFormsDelegateStub::GetValue(nsAString& aValue) 
> +{
> +  aValue = EmptyString();

Return null if there's no bound node?

> +nsXFormsDelegateStub::GetHasBoundNode(PRBool *aHasBoundNode)
> +{
> +  *aHasBoundNode = (mBoundNode != nsnull);

*aHasBoundNode = mBoundNode ? PR_TRUE : PR_FALSE;
instead to avoid explicit nsnull check?

> +++ xforms.xml	2005-06-09 17:37:48.000000000 +0300

Include the Mozilla copyright header.

Comment 37

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

Attachment: for review 4

I wanted to keep void disable(in boolean disable) in nsIXFormsUIWidget.

Comment 38

[Allan Beaufour]

(In reply to comment #37)
> Created an attachment (id=186257) [edit]
> for review 4
> 
> I wanted to keep void disable(in boolean disable) in nsIXFormsUIWidget.

Why?

Comment 39

[Allan Beaufour]

My version of the calculator that does not use table for layout fails to fresh
the controls when we change the appearance:
http://lab.cph.novell.com/~abeaufour/xforms/calc_svg.xhtml

(this is not new, it was in attachment 185798 [details] [diff] [review] too)

Comment 40

[timeless]

i don't think a warning for when you're running out of memory is particularly
useful. just deal with it.

Comment 41

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

Attachment: for review 4 (up-to-date)

Comment 42

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

Comment on attachment 186847 [details] [diff] [review]
for review 4 (up-to-date)

I don't remember now who is 
on vacation and when...but
maybe Doron could look at this too.

Comment 43

[Allan Beaufour]

(In reply to comment #42)
> (From update of attachment 186847 [details] [diff] [review] [edit])
> I don't remember now who is 
> on vacation and when...but
> maybe Doron could look at this too.
 
Sorry for the delay... I was in Amsterdam last week, etc. etc. But again, a
textual description of what you did from r3 to r4 would have been nice.

Comment 44

[Doron Rosenberg (IBM)]

Why for example does focus() return a boolean?  It is always returned as true...

Comment 45

[Allan Beaufour]

Comment on attachment 186847 [details] [diff] [review]
for review 4 (up-to-date)

* I will keep saying this: Please include a brief description of what has
  happened between revisions

* Nice with some good comments, including links!

* You seem to have fixed it all, as far as I can see. I haven't had time to
push it through all my tests, but not that much has changed, so I hope it still
works :)

>Index: nsXFormsModelElement.cpp
>===================================================================

>+nsPostRefresh::~nsPostRefresh()
>+{
>+#ifdef DEBUG_smaug
>+  printf("~nsPostRefresh\n");
>+#endif
>+  --sRefreshing;
>+  if (sPostRefreshList && !sRefreshing) {
>+    while (sPostRefreshList->Count()) {
>+      // Iterating from last to first saves possibly few memcopies,
>+      // see nsVoidArray::RemoveElementsAt().

But why do you iterate? You delete the list at the end of the function. [1]

>+      PRInt32 last = sPostRefreshList->Count() - 1;
>+      nsIXFormsControl* control =
>+        NS_STATIC_CAST(nsIXFormsControl*, sPostRefreshList->ElementAt(last));
>+      sPostRefreshList->RemoveElementAt(last);
>+      if (control)
>+        control->Refresh();
>+    }
>+    delete sPostRefreshList;
>+    sPostRefreshList = nsnull;
>+  }
>+}

>Index: nsXFormsDelegateStub.h
>===================================================================

>+enum nsRepeatState {
>+  eType_Unknown,
>+  eType_Template,
>+  eType_GeneratedContent,
>+  eType_NotApplicable
>+};

You didn't answer my question in comment 36:
And shouldn't this handling be in a class "closer" to nsIXFormsControl instead?
It should be valid for all controls.

But it's not terribly important, we can always move it.

r=me, with either a fix or a comment for [1] above.

And once again, sorry for the time it took me to get to this

Comment 46

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

I won't have time to watch tinderbox before Sunday night,
but if anyone wants to check this in, please do so.

The comment Allan wants could be
      // Iterating this way because refresh can lead to
      // additions/deletions in sPostRefreshList.

Comment 47

[Allan Beaufour]

(In reply to comment #46)
> I won't have time to watch tinderbox before Sunday night,
> but if anyone wants to check this in, please do so.

I do not have the time either :(

> The comment Allan wants could be
>       // Iterating this way because refresh can lead to
>       // additions/deletions in sPostRefreshList.

Exactly.

Comment 48

[Doron Rosenberg (IBM)]

I have time today, will take care of it

Comment 49

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

checked in