235866 - Add XULElement.builder.refresh() and automatically update composite datasource when datasource="" attribute is changed

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Benjamin Smedberg]

In my side-quest to make mozilla XUL more usable from remote websites, I have
added a little code to allow authors to force-reload RDF datasources. In
addition,  xul templates will now update themself properly if you change the
datasources="" attribute.

Comment 1

[Benjamin Smedberg]

Attachment: add nsIXULTemplateBuilder.refresh

Comment 2

[Jan Varga [:janv]]

sounds good to me

Comment 3

[Nigel McFarlane]

I think chrome authors will always appreciate being able to choose either
the RDF factory objects or the underlying XPCOM interfaces. So can this be
permanent rather than a "temporary hack"? Especially since this change
implies an addition to the XUL AOM. You can't add that and then take it
away again.

This fix is also part-way to the 'refresh' request in bug 232710.

Question: why isn't this functionality merely a Boolean argument
to the builder's rebuild() method? After all, both refresh() and
rebuild() are methods on the builder object, and both cause a
rebuild. Would:

New                       Old
builder.rebuild()      == builder.rebuild()  // no RDF reload
builder.rebuild(false) == builder.rebuild()  // no RDF reload
builder.rebuild(true)  == builder.refresh()  // RDF reloaded

be a more compact signature notation?

- N.

Comment 4

[Benjamin Smedberg]

*** Bug 230825 has been marked as a duplicate of this bug. ***

Comment 5

[Benjamin Smedberg]

(In reply to comment #3)
> the RDF factory objects or the underlying XPCOM interfaces. So can this be
> permanent rather than a "temporary hack"? Especially since this change
> implies an addition to the XUL AOM. You can't add that and then take it

Would you please stop using uncommon acronyms like "AOM", at least in my bugs?
It may be a computer-science term, but it's not used by mozilla developers.

The builder is really not the right place for this method. However, until RDF is
remotely scriptable this is the best place for me to put this method. I can
certainly add it and then take it away later when the correct implementations
become scriptable. If the method becomes part of common usage, we can decide
later that it's worth keeping as backwards-compatible hack; but it's not frozen.

> Question: why isn't this functionality merely a Boolean argument
> to the builder's rebuild() method? After all, both refresh() and

Because IDL doesn't allow default arguments, and rebuild is a semantically
different operation that reloading the underlying datasources. Normally
rebuild() is only needed if you change the <template><rule> structure, or if the
datasource doesn't fire notifications correctly.

Comment 6

[Nigel McFarlane]

AOM is not CompSci, it is apparently defined here:
http://www.mozilla.org/xpfe/aom/AOM.html
It is not my favorite acronym, either.

To clarify: there are three places for JS access to underlying
functionality: JS host objects like RDF, XPCOM objects via the
Components nameservice, and objects in the AOM object tree, like
window.document. It is the last that I question. There,
methods do support default values, eg window.open().

I never said "frozen". I just note that decorating the BOM or
AOM is generally an accretion process and it is very hard to
remove a feature once it has been added, since people become
accustomed to using it.

Perhaps I read you wrong. Is refresh() not a superset of the
functionality of rebuild(), then? The two are somehow orthogonal?

If refresh() only caused a datasource to both reload and spew
notifications of all changes, then I agree with you. But you
remark implies that datasources can't be relied on for that (yet?),
and so refresh() must force the rebuild() directly.

- N.

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 142456 [details] [diff] [review]
add nsIXULTemplateBuilder.refresh

>Index: src/nsXULTemplateBuilder.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/content/xul/templates/src/nsXULTemplateBuilder.cpp,v
>retrieving revision 1.292
>diff -p -u -8 -r1.292 nsXULTemplateBuilder.cpp
>--- src/nsXULTemplateBuilder.cpp	9 Feb 2004 22:48:40 -0000	1.292
>+++ src/nsXULTemplateBuilder.cpp	27 Feb 2004 21:04:09 -0000
>@@ -224,36 +225,71 @@ nsXULTemplateBuilder::GetDatabase(nsIRDF
>     NS_IF_ADDREF(*aResult = mDB.get());
>     return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsXULTemplateBuilder::Rebuild()
> {
>     PRInt32 i;
>-

Try not to make spurious whitespace changes unless there's some style cleanup
happening.

Looks good other than that.

Comment 8

[Benjamin Smedberg]

checked in