http://gwt-code-reviews.appspot.com/62808/diff/31/45 File xpcom/Preferences.h (right):
http://gwt-code-reviews.appspot.com/62808/diff/31/45#newcode22 Line 22: #include "nsISupports.h" On 2009/08/28 17:37:21, jat wrote: > On 2009/08/28 16:30:34, jaimeyap wrote: > > These headers get pulled in from the include path set in the make file. Some > of > > these headers seem only to exist in gecko-1.9.1/include. > > > > Means that if you are building for gecko-1.9.0 that the build would fail (as > it > > does on my mac). > It should be in plugin-sdks -- can you svn up to make sure it is current? On my > Mac, where I built ff2, ff3, and ff35 without any problems, I have: > $ find plugin-sdks -name nsIObserver.h > plugin-sdks/gecko-sdks/gecko-1.8/include/nsIObserver.h > plugin-sdks/gecko-sdks/gecko-1.9.0/include/nsIObserver.h > plugin-sdks/gecko-sdks/gecko-1.9.1/include/nsIObserver.h > This is with plugin-sdks at r6019 and no locally modified files. svn upped and got them. thanks. http://gwt-code-reviews.appspot.com/62808/diff/31/51 File xpcom/prebuilt/extension/content/options.xul (right): http://gwt-code-reviews.appspot.com/62808/diff/31/51#newcode45 Line 45: <listbox id="listbox" rows="5"> On 2009/08/28 17:37:21, jat wrote: > On 2009/08/28 16:30:34, jaimeyap wrote: > > The id should probably say what this UI item is supposed to be representing. > Not > > simply the tag name. > Ok, so something like accesslist_listbox? camelCased, but yeah something along those lines. http://gwt-code-reviews.appspot.com/62808/diff/31/52 File xpcom/prebuilt/extension/content/prefScript.js (right): http://gwt-code-reviews.appspot.com/62808/diff/31/52#newcode39 Line 39: prefs.splice(idx, 1); On 2009/08/28 17:37:21, jat wrote: > On 2009/08/28 16:30:34, jaimeyap wrote: > > shouldnt this be prefs = prefs.splice(idx,1) > No, splice is a mutating method. The return value is actually an array of the > elements removed (except Navigator 4). ah. thats right :). http://gwt-code-reviews.appspot.com/62808/diff/31/52#newcode78 Line 78: prefs.QueryInterface(Components.interfaces.nsIPrefBranch2); On 2009/08/28 17:37:21, jat wrote: > On 2009/08/28 16:30:34, jaimeyap wrote: > > shouldnt you be using the return of this queryinterface? > The sample code I was following did not -- I assume it was simply checking to > make sure it implemented that interface, though I don't think it would throw an > exception anyway. > I think these should be nsIPrefBranch anyway, since in the JS code we don't use > the observer interface that nsIPrefBranch2 supplies. > So would something like this be ok: > prefs = prefs.QueryInterface(Components.interfaces.nsIPrefBranch); > if (!prefs) { > return []; > } It won't be null. QueryInterface should always just return prefs. I think if QueryInterface fails it throws Components.results.NS_ERROR_NO_INTERFACE Since JavaScript doesn't really have the notion of interfaces, you dont really need to use the return value (it really is just the same "prefs" object) of the QI call, but it is the expected way to use COM like apis. http://gwt-code-reviews.appspot.com/62808 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
