530488 - The xforms 'refresh' action crashes firefox in certain contexts

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[David A Thompson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5


The crash seems to be somewhat specific to the 'refresh' action since at least some other actions (message, revalidate, rebuild, recalculate) don't trigger the crash. A crash report doesn't seem to be generated.


xforms version: xforms 0.8.7pre



Reproducible: Always

Steps to Reproduce:
1. Load the test document xforms1.xhtml
2. Select a value for 'questiontype'

Actual Results:  
Firefox dies an inglorious death

Comment 1

[David A Thompson]

Attachment: test document for demonstrating crash with refresh action

Comment 2

[Sergey Reymerov]

Click by select item calls nsXFormsModelElement::Refresh() that (deep in calltree) calls nsXFormsRefreshElement::HandleSingleAction that just calls nsXFormsModelElement::Refresh(). This repeates til crash by stack overflow.

Comment 3

[Sergey Reymerov]

Attachment: Solution

Refresh action of form calls from refresh model (as I wrote before). So, I push refresh from refresh action into refresh-queue that makes normal runtime (without crashing).

Comment 4

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

Comment on attachment 428151 [details] [diff] [review]
Solution

Since also nsXFormsActionElement seems to do this, I guess this looks right.

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 428151 [details] [diff] [review]
Solution

looks fine with me

>-  return model->Refresh();
>+  model->RequestRefresh();
>+  return NS_OK;

why do you return NS_OK instead of RequestRefresh() return value?

Comment 6

[Sergey Reymerov]

Because RequestRefresh returns void.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> Because RequestRefresh returns void.

it doesn't

Comment 8

[Sergey Reymerov]

Attachment: patch

Indeed, RequestRefresh() returns NS_IMETHODIMP. So I've update patch.

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 428339 [details] [diff] [review]
patch

it might be unnecessary to rerequest all reviews for changes like these.

Comment 10

[alexander :surkov (:asurkov)]

landed http://hg.mozilla.org/xforms/rev/32ffdcc6f490