Closed
Bug 254600
Opened 21 years ago
Closed 19 years ago
nsIXULTemplateBuilder.refresh() shouldn't call .rebuild()
Categories
(Core :: XUL, enhancement)
Core
XUL
Tracking
()
RESOLVED
FIXED
People
(Reporter: benjamin, Assigned: bernhard.hermann)
Details
Attachments
(2 files)
1007 bytes,
patch
|
Details | Diff | Splinter Review | |
2.81 KB,
patch
|
Details | Diff | Splinter Review |
From an IRC conversation, nsIXULTemplateBuilder.refresh() shouldn't call
.rebuild(), because we're normally loading the datasource async from the
network, and the data has not yet updated when we try to rebuild, which can
cause odd problems.
<bsmedberg> we could try just removing the .rebuild() call from the .refresh() impl
<bsmedberg> and making callers call .rebuild() if they really need ti
<telpochyaotl> bsmedberg: if that worked that would be perfect
* bsmedberg looks for r=Pike sr=Neil
<Pike> granted
<Neil> bsmedberg: sorry, lost you a sec... where's .refresh()?
<bsmedberg> nsIXULTemplateBuilder I think
* bsmedberg added it before 1.6 to allow remote chrome to update cached RDF
datasources
<bsmedberg> Neil: I just want to remove this line:
http://lxr.mozilla.org/mozilla/source/content/xul/templates/src/nsXULTemplateBuilder.cpp#265
<bsmedberg> and return NS_OK instead
<telpochyaotl> bsmedberg: once that's done, how do i check it out? (i just have
an apt-getted mozilla)
<Pike> bsmedberg: if we'd want much, we could QI to nsIRDFXMLSink and add an
observer, which, after being called enough, would call rebuild()
<Neil> bsmedberg: refreshing won't actually cause notifications, will it?
<bsmedberg> Neil: yes, it does
<bsmedberg> Neil: we use a mark and sweep algorithm
<Pike> I'm not talking about an nsIRDFObserver, btw, but an nsIRDFXMLSinkObserver
<Neil> bsmedberg: ah, so the sweep clears out any dead assertions?
<bsmedberg> right
<Neil> bsmedberg: great. sr=me
Reporter | ||
Comment 1•21 years ago
|
||
Reporter | ||
Comment 2•21 years ago
|
||
I have committed attachment 155367 [details] [diff] [review]. I'm going to leave this bug open for Pike's
suggestion to install an nsIRDFXMLSink observer.
Reporter | ||
Updated•20 years ago
|
Assignee: benjamin → axel
Assignee | ||
Comment 3•19 years ago
|
||
This is an implementation of the suggestion by bsmedberg/Pike to do a Rebuild() after an async Refresh() via an nsIRDFXMLSink.
Works smoothly with seamonkey-1.0.2-sources. Unfortunately I wasn't able to test against the CVS sources, because I can't get them to compile. The patch is against the current CVS though, and I strongly suppose it to work.
Reporter | ||
Updated•19 years ago
|
Assignee: axel → bernhard.hermann
Reporter | ||
Updated•19 years ago
|
Attachment #228804 -
Flags: review?(enndeakin)
Assignee | ||
Comment 5•19 years ago
|
||
(In reply to comment #4)
> #define @ asm { NOP; }
>
uh... like.. oops.
Comment 6•19 years ago
|
||
The template builder should be notified of changes while the datasource is loading already, ingoring bugs in handling certain cases. I'm not convinced that refresh should always rebuild afterwards.
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•19 years ago
|
||
(In reply to comment #6)
> The template builder should be notified of changes while the datasource is
> loading already, ingoring bugs in handling certain cases. I'm not convinced
> that refresh should always rebuild afterwards.
>
We were thinking it doesn't work in JS, that's why I wrote this "enhancement". But apparently {Pike,bsmedberg}'s suggestion isn't needed. Shall I resolve the bug?
I would add a boolean to Refresh(), so it becomes something like Refresh(boolean rebuildAfterwards). But it would require a change to the header of the method, which is rather inconvenient, isn't it?
Assignee | ||
Comment 8•19 years ago
|
||
no responses, bug closed to reduce clutter
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Attachment #228804 -
Flags: review?(enndeakin)
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
You need to log in
before you can comment on or make changes to this bug.
Description
•