Status

Core Graveyard
XForms
RESOLVED FIXED
10 years ago
11 months ago

People

(Reporter: aaronr, Assigned: aaronr)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

Trunk
x86
Windows XP
fixed1.8.0.12, fixed1.8.1.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

1.61 KB, application/xhtml+xml
Details
3.96 KB, patch
surkov
: review+
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
Created attachment 262726 [details]
testcase

my patch for bug 377878 had a bug in it where I didn't correctly convert the nsisupports* back over to a nsIModelElementPrivate*.  I also previously forgot to handle the recalculate, revalidate and refresh events.  Doh!

Comment 1

10 years ago
And I reviewed bug 377878, oops.
(Assignee)

Comment 2

10 years ago
Created attachment 262727 [details] [diff] [review]
patch
Attachment #262727 - Flags: review?(surkov.alexander)
(Assignee)

Updated

10 years ago
Attachment #262727 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 3

10 years ago
(In reply to comment #1)
> And I reviewed bug 377878, oops.
> 

Nah, the bug wasn't in the patch.  The bug was the fact that I changed what went into the hashtable but I didn't change how we got the value back inside the ennumeration function.

Comment 4

10 years ago
Comment on attachment 262727 [details] [diff] [review]
patch


> PR_STATIC_CALLBACK(PLDHashOperator) DoDeferredActions(nsISupports * aModel, 
>                                                       PRUint32 aDeferred,
>                                                       void * data)
> {
>   if (aModel && aDeferred) {
>-    nsCOMPtr<nsIModelElementPrivate> model =
>-      NS_STATIC_CAST(nsIModelElementPrivate*, aModel);
>+    nsCOMPtr<nsISupports> modelSupp =
>+      NS_STATIC_CAST(nsISupports*, aModel);

aModel is already nsISupports* so no need to cast.

I think the whole problem could be fixed in some better way, but let's do this now for 0.8.
(The better way might be to use nsIModelElementPrivate as the key in the hashtable)
Attachment #262727 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 5

10 years ago
(In reply to comment #4)
> (From update of attachment 262727 [details] [diff] [review])
> 
> > PR_STATIC_CALLBACK(PLDHashOperator) DoDeferredActions(nsISupports * aModel, 
> >                                                       PRUint32 aDeferred,
> >                                                       void * data)
> > {
> >   if (aModel && aDeferred) {
> >-    nsCOMPtr<nsIModelElementPrivate> model =
> >-      NS_STATIC_CAST(nsIModelElementPrivate*, aModel);
> >+    nsCOMPtr<nsISupports> modelSupp =
> >+      NS_STATIC_CAST(nsISupports*, aModel);
> 
> aModel is already nsISupports* so no need to cast.
> 

I thought we had to cast to a comptr to do a QI.  But you are right, it works w/o it.

> I think the whole problem could be fixed in some better way, but let's do this
> now for 0.8.
> (The better way might be to use nsIModelElementPrivate as the key in the
> hashtable)
> 

That's what the problem was to begin with.  QI'ing the same nsIDOMElement model to a nsIModelElementPrivate was giving back different nsIModelElementPrivate values, so looking for the values in the hashtable didn't work.
(Assignee)

Comment 6

10 years ago
Created attachment 262731 [details] [diff] [review]
patch2

fixes qi nit olli found
Attachment #262727 - Attachment is obsolete: true
Attachment #262731 - Flags: review?(surkov.alexander)
Attachment #262727 - Flags: review?(surkov.alexander)

Updated

10 years ago
Attachment #262731 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 7

10 years ago
checked into trunk, 1.8 and 1.8.0
Keywords: fixed1.8.0.12, fixed1.8.1.4
(Assignee)

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.