If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

BookmarksMenu items from distribution.ini do not get added to Bookmarks Menu

VERIFIED FIXED in Firefox 3.5

Status

()

Firefox
General
--
major
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: kev, Assigned: rc)

Tracking

({verified1.9.0.2})

Trunk
Firefox 3.5
verified1.9.0.2
Points:
---
Bug Flags:
blocking1.9.0.1 -
blocking1.9.0.2 +
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MU+])

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
Created attachment 323093 [details]
distribution.ini for testcase

When any link items are defined in the [BookmarksMenu] section of <appdir>/distribution/distribution.ini, the following error is thrown on profile creation, and no bookmark customizations appear in either the Bookmarks Toolbar or Bookmarks menu. 

Error: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsINavBookmarksService.insertBookmark]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox/modules/distribution.js :: DIST_parseBookmarksSection :: line 189"  data: no]
Source File: file:///C:/Program%20Files/Mozilla%20Firefox/modules/distribution.js
Line: 189 

Removing all link items from the [BookmarksMenu] section permits the customizations made in the [BookmarksToolbar] section to be applied, but any other customizations in the [BookmarksMenu] section (i.e. folders) will not appear in the Bookmarks Menu.

Steps to reproduce:

- Create a "distribution" directory off the application directory
- Copy the "distribution.ini for testcase" attachment to the distribution directory
- Start Firefox3 with a new profile

Expected results: Firefox 3 with a test link and folder in the Bookmarks Toolbar, and a test link and folder in the Bookmarks Menu

Actual results: Firefox 3 with default Bookmarks Toolbar and Menu

(removing the [BookmarksMenu] section and restarting with a new profile will result in the BookmarksToolbar being populated as expected with one link item and one folder item)
the call to insertBookmark call in distribution.js looks correct, and that API has not changed in a very long time. i'm not familiar with how the ini is being parsed etc, so cc'ing thunder, author of this code.

Comment 2

9 years ago
The distribution module is using 'bmSvc.bookmarksRoot' to get the menu root (where 'bmSvc' is nsINavBookmarksService), but that property was renamed to 'bookmarksMenuFolder' a while back:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsINavBookmarksService.idl&branch=&root=/cvsroot&subdir=/mozilla/toolkit/components/places/public&command=DIFF_FRAMESET&rev1=1.49&rev2=1.50
Status: NEW → ASSIGNED
(Reporter)

Updated

9 years ago
Flags: blocking1.9.0.1?

Updated

9 years ago
Blocks: 413454
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
Whiteboard: [MU+]
(Assignee)

Comment 3

9 years ago
Created attachment 328045 [details] [diff] [review]
[checked-in] distribution menu folder fix

it's easy when someone tells you how
Assignee: nobody → rcampbell
Attachment #328045 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #328045 - Flags: review? → review?(thunder)
(Assignee)

Comment 4

9 years ago
we'll likely also need approval flags on this, leaving that up to the module owner.

Comment 5

9 years ago
Comment on attachment 328045 [details] [diff] [review]
[checked-in] distribution menu folder fix

Thanks!  Looks good.
Attachment #328045 - Flags: review?(thunder) → review+
(Assignee)

Updated

9 years ago
Attachment #328045 - Flags: approval1.9.0.2?

Updated

9 years ago
Attachment #328045 - Flags: approval1.9.0.2? → approval1.9.0.2+

Updated

9 years ago
Flags: blocking1.9.0.2+

Updated

9 years ago
Attachment #323093 - Attachment mime type: application/octet-stream → text/plain
Attachment #328045 - Flags: review?(benjamin)
Need review from benjamin, unless mconnor/bsmedberg want to update the reviewer page to make thunder an appropriate reviewer.
(Reporter)

Comment 7

9 years ago
I'd ask that Benjamin review, please.
Does browser/modules/Distribution.jsm need patching too? Which one is actually used? (Why do we have both, when they appear to be identical?)
Looks like all of /modules is unused, disregard my comment, I guess!

(Should probably fix the other references to this no longer existing property, right? http://lxr.mozilla.org/seamonkey/search?string=.bookmarksRoot )
(Assignee)

Comment 10

9 years ago
(In reply to comment #9)

> (Should probably fix the other references to this no longer existing property,
> right? http://lxr.mozilla.org/seamonkey/search?string=.bookmarksRoot )

that sounds like a good idea.

Updated

9 years ago
Attachment #328045 - Flags: review?(benjamin)
Should we remove the .jsm file if it's not used?  I didn't add it, I'm not sure why it's there.
(Assignee)

Updated

9 years ago
Attachment #328045 - Flags: review?(benjamin)
(Assignee)

Updated

9 years ago
Attachment #328045 - Flags: review?(benjamin) → review?(gavin.sharp)
(Assignee)

Comment 12

9 years ago
(In reply to comment #11)
> Should we remove the .jsm file if it's not used?  I didn't add it, I'm not sure
> why it's there.

I'm all for removing unnecessary files.

Moving the review request from bsmedberg to gavin per Benjamin's discussion in IRC.
Attachment #328045 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 13

9 years ago
Comment on attachment 328045 [details] [diff] [review]
[checked-in] distribution menu folder fix

landed on cvs trunk

Checking in distribution.js;
/cvsroot/mozilla/browser/components/distribution.js,v  <--  distribution.js
new revision: 1.3; previous revision: 1.2
done
Attachment #328045 - Attachment description: distribution menu folder fix → [checked-in] distribution menu folder fix
(Assignee)

Comment 14

9 years ago
and pushed to mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
keyword: fixed1.9.0.2?
(Assignee)

Comment 16

9 years ago
oops. Thanks.
Keywords: fixed1.9.0.2
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.2pre) Gecko/2008080705 GranParadiso/3.0.2pre.   Testcase in the attachment yields the expected results from comment 1.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.0.2 → verified1.9.0.2
Target Milestone: Firefox 3.1 → Firefox 3.5
You need to log in before you can comment on or make changes to this bug.