1002070 - FUEL's BookmarksObserver uses the non-existent nsINavBookmarksObserver interface

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(no longer active)]

http://mxr.mozilla.org/mozilla-central/source/browser/fuel/src/fuelApplication.js#379

This code gets triggered when you open the web console in about:config under this stack:

0 <TOP LEVEL> ["file:///Users/ehsan/moz/src/obj-ff-dbg.noindex/dist/NightlyDebug.app/Contents/MacOS/browser/components/fuelApplication.js":379]
    this = [object BackstagePass @ 0x128135760 (native @ 0x126d8e2a0)]
1 anonymous(aObj = [object Object]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/toolkit/webconsole/utils.js":1063]
2 getMatchedProps_impl(aObj = [object Object], aMatch = "C",  = [object Object]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/toolkit/webconsole/utils.js":976]
3 getMatchedPropsInDbgObject(aDbgObject = [object Object], aMatch = "C") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/toolkit/webconsole/utils.js":944]
4 JSPropertyProvider(aDbgObject = [object Object], anEnvironment = null, aInputValue = "C", aCursor = 1, inputValue = null) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/toolkit/webconsole/utils.js":854]
5 anonymous(arguments = [object Object], null, "C", 1, null) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js":84]
6 WCA_onAutocomplete(aRequest = [object Object], frameActorId = [object Object]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/webconsole.js":800]
    this = [object Object]
7 DSC_onPacket(aPacket = [object Object]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js":1098]
    this = [object Object]
8 anonymous() ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/transport.js":279]
    this = function (/* arguments */) {
"use strict";

    try {
      return aHandler.apply(this, arguments);
    } catch (ex) {
      let who = "Handler function";
      if (aName) {
        who += " " + aName;
      }
      exports.reportException(who, ex);
    }
  }

Comment 1

[Marco Bonardo [:mak]]

yep, sounds like a typo, should be nsINavBookmarkObserver, these should be fixed

http://mxr.mozilla.org/mozilla-central/search?string=nsINavBookmarksObserver

Comment 2

[Lars Swientek]

i will work on this, is there any additional information i need?

Comment 3

[Marco Bonardo [:mak]]

Hello, which kind of information are you looking for?

General documentation on how to contribute and make a build is here:
https://developer.mozilla.org/en-US/docs/Introduction
Coding style information is here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style

Comment 0 describes how to reproduce the issue, and comment 1 has the likely cause (after fixing it should be enough to check comment 0 is not reproduced anymore).

Considered the kind of code and change a test is unlikely to be needed here.

Comment 4

[Lars Swientek]

Attachment: PATCH.patch

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8428814 [details] [diff] [review]
PATCH.patch

Review of attachment 8428814 [details] [diff] [review]:
-----------------------------------------------------------------

please remember to add r=mak to the commit message, then you can set checkin-needed keyword into the keywords field up here.

I'm not going to issue a try run for this change, it's quite trivial and very very unlikely to cause orange.

Comment 6

[Lars Swientek]

Comment on attachment 8428814 [details] [diff] [review]
PATCH.patch

># HG changeset patch
># Parent 41a54c8add09fe29b472fb4b7787b60a45dca1c9
># User Lars Swientek <[e-mail]>
>Bug 1002070 - Change nsINavBookmarksObserver to nsINavBookmarkObserver so that
>right interface is accessed; r=mak
>
>diff --git a/browser/fuel/src/fuelApplication.js b/browser/fuel/src/fuelApplication.js
>--- a/browser/fuel/src/fuelApplication.js
>+++ b/browser/fuel/src/fuelApplication.js
>@@ -371,17 +371,17 @@ BookmarksObserver.prototype = {
>   addRootListener: function addRootListener(aEvent, aListener) {
>     this._rootEvents.addListener(aEvent, aListener);
>   },
> 
>   removeRootListener: function removeRootListener(aEvent, aListener) {
>     this._rootEvents.removeListener(aEvent, aListener);
>   },
> 
>-  QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarksObserver,
>+  QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarkObserver,
>                                          Ci.nsISupportsWeakReference])
> };
> 
> //=================================================
> // Bookmark implementation
> //
> // Bookmark event listeners are stored in BookmarksObserver, not in the
> // Bookmark objects themselves.  Thus, you don't have to hold on to a Bookmark
>@@ -490,17 +490,17 @@ Bookmark.prototype = {
> 
>   onItemMoved: function(aId, aOldParent, aOldIndex, aNewParent, aNewIndex) {
>     if (aId == this._id) {
>       this._parent = new BookmarkFolder(aNewParent, Utilities.bookmarks.getFolderIdForItem(aNewParent));
>     }
>   },
> 
>   QueryInterface: XPCOMUtils.generateQI([Ci.fuelIBookmark,
>-                                         Ci.nsINavBookmarksObserver,
>+                                         Ci.nsINavBookmarkObserver,
>                                          Ci.nsISupportsWeakReference])
> };
> 
> 
> //=================================================
> // BookmarkFolder implementation
> //
> // As with Bookmark, events on BookmarkFolder are handled by the
>@@ -662,17 +662,17 @@ BookmarkFolder.prototype = {
> 
>   onItemMoved: function bf_onItemMoved(aId, aOldParent, aOldIndex, aNewParent, aNewIndex) {
>     if (this._id == aId) {
>       this._parent = new BookmarkFolder(aNewParent, Utilities.bookmarks.getFolderIdForItem(aNewParent));
>     }
>   },
> 
>   QueryInterface: XPCOMUtils.generateQI([Ci.fuelIBookmarkFolder,
>-                                         Ci.nsINavBookmarksObserver,
>+                                         Ci.nsINavBookmarkObserver,
>                                          Ci.nsISupportsWeakReference])
> };
> 
> //=================================================
> // BookmarkRoots implementation
> function BookmarkRoots() {
> }
>

Comment 7

[Lars Swientek]

Attachment: Patch(v2).patch

Comment 8

[Lars Swientek]

ok, thank you for your help and sorry for the comment on my attachment i thought it would be possible to change it that way

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/2773518645bf

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/2773518645bf