The xforms 'refresh' action crashes firefox in certain contexts

RESOLVED FIXED

Status

Core Graveyard
XForms
--
critical
RESOLVED FIXED
8 years ago
a year ago

People

(Reporter: David A Thompson, Assigned: Sergey Reymerov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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
(Reporter)

Comment 1

8 years ago
Created attachment 414003 [details]
test document for demonstrating crash with refresh action

Updated

8 years ago
Blocks: 539184
(Assignee)

Comment 2

8 years ago
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.
(Assignee)

Comment 3

8 years ago
Created attachment 428151 [details] [diff] [review]
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).
Attachment #428151 - Flags: review?(alexey.gaidukov)
(Assignee)

Updated

8 years ago
Attachment #428151 - Flags: review?(Olli.Pettay)
(Assignee)

Updated

8 years ago
Attachment #428151 - Flags: review?(alexey.gaidukov) → review?(surkov.alexander)
Comment on attachment 428151 [details] [diff] [review]
Solution

Since also nsXFormsActionElement seems to do this, I guess this looks right.
Attachment #428151 - Flags: review?(Olli.Pettay) → review+

Comment 5

8 years ago
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?
Attachment #428151 - Flags: review?(surkov.alexander) → review+

Updated

8 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 6

8 years ago
Because RequestRefresh returns void.

Comment 7

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

it doesn't
(Assignee)

Comment 8

8 years ago
Created attachment 428339 [details] [diff] [review]
patch

Indeed, RequestRefresh() returns NS_IMETHODIMP. So I've update patch.
Attachment #428151 - Attachment is obsolete: true
Attachment #428339 - Flags: review?(surkov.alexander)
(Assignee)

Updated

8 years ago
Attachment #428339 - Flags: review?(Olli.Pettay)

Comment 9

8 years ago
Comment on attachment 428339 [details] [diff] [review]
patch

it might be unnecessary to rerequest all reviews for changes like these.
Attachment #428339 - Flags: review?(surkov.alexander) → review+
(Assignee)

Updated

8 years ago
Attachment #428339 - Flags: review?(Olli.Pettay)

Comment 10

8 years ago
landed http://hg.mozilla.org/xforms/rev/32ffdcc6f490
Assignee: nobody → sergeyreym
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
OS: Linux → All
Hardware: x86 → All
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.