Closed Bug 421235 Opened 16 years ago Closed 16 years ago

FUEL: Add support for new bookmark roots

Categories

(Firefox :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta5

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

(Keywords: addon-compat)

Attachments

(1 file, 1 obsolete file)

When originally landed, FUEL supported a single bookmark root (menu) because that's the only root that existed. Not long after that, several new bookmark roots were added: tags, toolbar and unfiled.

We should add support to FUEL to get access to the new roots.
Attached patch Adds nsIBookmarkRoots and tests (obsolete) — Splinter Review
This patch adds a nsIBookmarkRoots interface to hold the various bookmark roots. Each root getter simply returns a nsIBookmarkFolder for the specific root.

Tests are added to perform some basic "is the root there" tests. Since each root returns a nsIBookmarkFolder the main testing code is only executed once on the "menu" bookmark root.
Assignee: nobody → mark.finkle
Status: NEW → ASSIGNED
Attachment #307651 - Flags: review?(gavin.sharp)
Comment on attachment 307651 [details] [diff] [review]
Adds nsIBookmarkRoots and tests

>Index: browser/fuel/public/fuelIApplication.idl

>+function BookmarkRoots() {
>+  _menu = null;
>+  _toolbar = null;
>+  _tags = null;
>+  _unfiled = null;

r=me with these removed and these:

>+    if (this._menu == null)

turned into just |if (!this._menu)| and

http://lxr.mozilla.org/seamonkey/source/browser/base/content/test/browser_getshortcutoruri.js

fixed to use something other than just Application.bookmarks

and with the browser test suite run and other tests fixed as needed!
Attachment #307651 - Flags: review?(gavin.sharp) → review+
Here is the patch to land with changes per gavin's review. Pulling r+ forward.
Attachment #307651 - Attachment is obsolete: true
Attachment #308090 - Flags: review+
Comment on attachment 308090 [details] [diff] [review]
v2 - same as first patch with review changes

This patch addresses some important missing functionality with the FUEL bookmark wrappers. There is a chance it will break some Fx3.0b extensions, but the changes are needed to allow access to all the bookmark roots. There is no clean way to not break any current consumers.

I have a bug filed to scan the AMO source for possible breakage and I'll be blogging about the change when it lands.

There are currently no internal Firefox consumers that are affected.
Attachment #308090 - Flags: approval1.9?
Keywords: late-compat
Target Milestone: --- → Firefox 3 beta5
Comment on attachment 308090 [details] [diff] [review]
v2 - same as first patch with review changes

a1.9+=damons
Attachment #308090 - Flags: approval1.9? → approval1.9+
Checking in browser/base/content/test/browser_getshortcutoruri.js;
/cvsroot/mozilla/browser/base/content/test/browser_getshortcutoruri.js,v  <--  browser_getshortcutoruri.js
new revision: 1.2; previous revision: 1.1
done
Checking in browser/fuel/public/fuelIApplication.idl;
/cvsroot/mozilla/browser/fuel/public/fuelIApplication.idl,v  <--  fuelIApplication.idl
new revision: 1.12; previous revision: 1.11
done
Checking in browser/fuel/src/fuelApplication.js;
/cvsroot/mozilla/browser/fuel/src/fuelApplication.js,v  <--  fuelApplication.js
new revision: 1.27; previous revision: 1.26
done
Checking in browser/fuel/test/browser_Bookmarks.js;
/cvsroot/mozilla/browser/fuel/test/browser_Bookmarks.js,v  <--  browser_Bookmarks.js
new revision: 1.3; previous revision: 1.2
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: