Closed
Bug 421235
Opened 16 years ago
Closed 16 years ago
FUEL: Add support for new bookmark roots
Categories
(Firefox :: General, defect)
Tracking
()
RESOLVED
FIXED
Firefox 3 beta5
People
(Reporter: mfinkle, Assigned: mfinkle)
Details
(Keywords: addon-compat)
Attachments
(1 file, 1 obsolete file)
8.56 KB,
patch
|
mfinkle
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
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 2•16 years ago
|
||
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•16 years ago
|
||
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•16 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•16 years ago
|
Keywords: late-compat
Assignee | ||
Updated•16 years ago
|
Target Milestone: --- → Firefox 3 beta5
Comment 5•16 years ago
|
||
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•16 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
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•