FUEL: Add support for new bookmark roots

RESOLVED FIXED in Firefox 3 beta5

Status

()

Firefox
General
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

({addon-compat})

Trunk
Firefox 3 beta5
x86
Windows XP
addon-compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
(Assignee)

Comment 1

10 years ago
Created attachment 307651 [details] [diff] [review]
Adds nsIBookmarkRoots and tests

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+
(Assignee)

Comment 3

10 years ago
Created attachment 308090 [details] [diff] [review]
v2 - same as first patch with review changes

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+
(Assignee)

Comment 4

10 years ago
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?
(Assignee)

Updated

10 years ago
Keywords: late-compat
(Assignee)

Updated

10 years ago
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+
(Assignee)

Comment 6

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.