355100 - Remove XTF visuals

Status: RESOLVED FIXED

(Core Graveyard :: XTF, defect)

Description

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

.

Comment 1

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

Attachment: Remove XTF visuals, leaving only nsIXTFElement/nsIXTFElementWrapper

Alex, what do you think about this?

Comment 2

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

Attachment: For XForms

This is needed for XForms.

Comment 3

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

Comment on attachment 240941 [details] [diff] [review]
Remove XTF visuals, leaving only nsIXTFElement/nsIXTFElementWrapper

This patch doesn't include the to-be-removed-files in layout/xtf

Comment 4

[Boris Zbarsky [:bzbarsky]]

There's more code in frame constructor that could go.  For example, I think all the mInsertionContent/CreateInsertionPointChildren/PushAnonymousContentCreator thing is XTF-only, as well as some of the extra args to CreateAnonymousFrames.

Might be worth looking at the CVS log for frame constructor to find all the code that XTF has its tentacles in.

Comment 5

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

Attachment: Remove more XTF related code

This backs out also Bug 269023 and Bug 270136.
(The patch doesn't contain layout/xtf)

Comment 6

[Alex Fritze]

Comment on attachment 241043 [details] [diff] [review]
Remove more XTF related code

Looks good to me.

Comment 7

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

Comment on attachment 241043 [details] [diff] [review]
Remove more XTF related code

Bryner, could you look at this too. Especially the layout part, since I'm not too familiar with that and basically I was just trying to back out your patches.

(And if things look ok, I'll ask sr?)

Comment 8

[aaronr]

Comment on attachment 240942 [details] [diff] [review]
For XForms


>Index: extensions/xforms/nsXFormsStubElement.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsStubElement.cpp,v
>retrieving revision 1.11
>diff -u -8 -p -r1.11 nsXFormsStubElement.cpp
>--- extensions/xforms/nsXFormsStubElement.cpp	24 May 2006 07:49:02 -0000	1.11
>+++ extensions/xforms/nsXFormsStubElement.cpp	2 Oct 2006 15:19:40 -0000

> 
> NS_IMETHODIMP
> nsXFormsStubElement::GetScriptingInterfaces(PRUint32 *aCount, nsIID ***aArray)
> {
>-  return nsXFormsUtils::CloneScriptingInterfaces(sScriptingIIDs,
>-                                                 NS_ARRAY_LENGTH(sScriptingIIDs),
>-                                                 aCount, aArray);
>+  *aCount = 0;
>+  *aArray = nsnull;
>+  return NS_OK;
> }

should probably test the pointers before doing this, right?  Or is it safe to assume that they are good?

r=me

Comment 9

[aaronr]

Olli, please check out http://www.mozilla.org/projects/xforms/samples/insurance_form/app_certificates.xhtml with these two patches built.  We lose our nice table layout.  I don't see anything in your xforms changes to explain that, so maybe is caused by the other stuff?  I dunno.

Comment 10

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

Attachment: For XForms, v2

I had removed aWrapper->SetClassAttributeName(nsXFormsAtoms::clazz)
Now the example should look ok.

Comment 11

[aaronr]

(In reply to comment #10)
> Created an attachment (id=241314) [edit]
> For XForms, v2
> 
> I had removed aWrapper->SetClassAttributeName(nsXFormsAtoms::clazz)
> Now the example should look ok.
> 

form works for me now.  Thanks.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 241043 [details] [diff] [review]
Remove more XTF related code

>Index: content/xtf/src/nsXTFElementWrapper.cpp
>+nsXTFElementWrapper::GetInterfaces(PRUint32* aCount, nsIID*** aArray)

What part of the old code is this replacing?

>+    if (!iids[i]) {
>+      for (PRUint32 j = 0; j < i; ++j) {
>+        nsMemory::Free(iids[j]);
>+      }
>+      nsMemory::Free(baseArray);
>+      nsMemory::Free(xtfArray);
>+      nsMemory::Free(iids);

But doesn't that leak the actual ids in xtfArray and baseArray?  I'm not sure how those are supposed to be deallocated in general, but...

>+    if (!iids[i]) {

Similar here.

>+  nsMemory::Free(baseArray);
>+  nsMemory::Free(xtfArray);

And here.

sr=bzbarsky with that addressed.

Comment 13

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

(In reply to comment #12)
> (From update of attachment 241043 [details] [diff] [review] [edit])
> >Index: content/xtf/src/nsXTFElementWrapper.cpp
> >+nsXTFElementWrapper::GetInterfaces(PRUint32* aCount, nsIID*** aArray)
> 
> What part of the old code is this replacing?
> 

Not really replacing, but combining the functionality of XTFBindableElement and other XTFElements. So XTF elements can define which interfaces to expose (like currently in XTFElements), but the 
default interfaces are there too (like in XTFBindableElement).

> But doesn't that leak the actual ids in xtfArray and baseArray?  I'm not sure
> how those are supposed to be deallocated in general, but...
> 

I'll use NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY.
(That is used also in nsDOMClassInfo)
nsDOMClassInfo

Comment 14

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

Checked in and removed the xtfvisual files from content/xtf and layout/xtf. code savings ~24000-28000.