242212 - Remove useless nsISecurityCheckedComponent code from nsXULTemplateBuilder

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[timeless]

The code doesn't seem to be needed, doron has tested mozilla and content samples
without regressions

Comment 1

[timeless]

Attachment: remove code

Comment 2

[Jan Varga [:janv]]

Comment on attachment 147401 [details] [diff] [review]
remove code

r=varga, but please check with peterv, jst or any other security guy

Comment 3

[Boris Zbarsky [:bzbarsky]]

This absolutely needs review from one of the people mentioned in comment 2.

Comment 4

[Mike Shaver (:shaver emeritus)]

Comment on attachment 147401 [details] [diff] [review]
remove code

sr=shaver.

Comment 5

[Johnny Stenback (:jst)]

Pike, does this affect your RDF-from-untrusted-sources changes?

Comment 6

[Axel Hecht]

I wouldn't know. I just checked with doron on irc, he did not check any remote
templates. (Which isn't easy with xulplanet being down and all.)
I can't evaluate the implications of this change from mere looking, I'd prefer
to see a detailed list of stuff that got tested before and after this patch.
Anyway, nsIXULTemplateBuilder was not part of my scriptable-rdf patch.

Comment 7

[Doron Rosenberg (IBM)]

I did a quick smoketest, making sure I hit chrome rdf/xul templates parts such
as bookmarks, email, etc.

Also ran a local xul template/rdf example I took off xulplanet and it worked fine.

Comment 8

[Johnny Stenback (:jst)]

Ok, I'm fine with removing this code as long as noone can think of a reason why
we'd need it. It's been there since version 1.1, so nothing very helpful in CVS
blame about this either.

Comment 9

[timeless]

mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp 	1.296
mozilla/content/xul/templates/src/nsXULTemplateBuilder.h 	1.21

Comment 10

[Neil Deakin]

This shouldn't affect chrome or the displaying of templates, but I'm pretty sure
this change would break somenode.builder.rebuild() and friends from remote xul.
That doesn't sound like the intention of this bug.

Comment 11

[Johnny Stenback (:jst)]

Doron, wanna whip up a testcase that tests if that's now broken? If it is, we'll
need to either put this code back, or give this class classinfo and say that
it's a "DOM object" and it's fine to access it from web content.

Comment 12

[Neil Deakin]

Testcase is at:
http://xulplanet.mozdev.org/tutorials/xultu/examples/builder-test.xul

Doron and AnnVanK confirm that the patch breaks builder.rebuild for remote content.

Comment 13

[Benjamin Smedberg]

Whoa! that's not good... builder method should definitely be available to
same-origin script.

Comment 14

[Johnny Stenback (:jst)]

timeless, wanna back out this change or go fix bug 242467?

Comment 15

[timeless]

i'd rather fix bug 242467. this isn't on the branch so would people please give
me until freeze for 1.8a is imminent before backing me out?

It looks like there are three derived impls of this creature, will it be enough
to add classinfo with DOM_OBJECT to nsXULTemplateBuilder?