Closed Bug 336857 Opened 18 years ago Closed 18 years ago

Places disabled build not populating bookmarks menu

Categories

(Firefox :: Bookmarks & History, defect)

x86
All
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: tor, Assigned: myk)

References

Details

(Keywords: fixed1.8.1, regression)

Attachments

(1 file)

A Firefox build from cvs trunk (evening, May 6th) with places disabled is not adding the bookmarks to the bookmarks menu.  The bookmarks are still intact and shown when "Organize Bookmarks..." is selected.
I see this on Windows too. A build from 2006-05-05-12 works OK, so the relevant checkins are these:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-05-05+12%3A00%3A00&maxdate=2006-05-05+23%3A00%3A00&cvsroot=%2Fcvsroot

Backing out the changes to browser-menubar.inc from bug 334471 gives me my bookmarks back.
Keywords: regression
OS: Linux → All
Are you building with Places disabled on the trunk or the branch?  Do you also not see folders in the menu, or is it just the bookmarks that are missing?

Taking, as this seems microsummaries-related if backing out the browser-menubar.inc changes fix it.
Assignee: nobody → myk
In my case, it's a home-made trunk build with Places disabled. Branch builds seem unaffected.

In "Bookmarks" menu, neither folders nor bookmarks are displayed, just the regular menuitems and the separator.

On the Bookmarks Toolbar, bookmarks and folders are displayed, but the folders are all empty. Overflowing bookmarks are also gone from the chevron menu.
Since it doesn't happen on the branch, this bug looks like a trunk regression in template processing that just happens to have become exposed from the new microsummaries code.  It may have something to do with the way microsummaries mixes simplified and extended syntax rules in a XUL template, which is supposed to work but doesn't appear to be well-exercised in the current codebase.

Neil Deakin has suggested instead defining a microsummary bookmark type so I can avoid mixing the syntaxes.  Here's a patch which does just that.  With this patch applied, bookmarks once again show up in the bookmarks menu, folders on the bookmarks toolbar, and the overflow menu.

Not sure who should review this; Neil perhaps?
Attachment #221115 - Flags: review?(enndeakin)
Comment on attachment 221115 [details] [diff] [review]
patch v1: works around problem with microsummary bookmark type

>+    // If we're setting the generator URI field, make sure the bookmark's
>+    // RDF type is set to the microsummary bookmark type.
>+    if (fieldName == FIELD_MICSUM_GEN_URI &&
>+        this._getField(bookmarkID, FIELD_RDF_TYPE) != VALUE_MICSUM_BOOKMARK) {
>+      if (this._hasField(bookmarkID, FIELD_RDF_TYPE)) {
>+        oldValue = this._getField(bookmarkID, FIELD_RDF_TYPE);

Could just call getField once there.

Patch looks OK though.
Attachment #221115 - Flags: review?(enndeakin) → review+
Attachment #221115 - Flags: superreview?(bugs)
Comment on attachment 221115 [details] [diff] [review]
patch v1: works around problem with microsummary bookmark type

sr=ben@mozilla.org
Attachment #221115 - Flags: superreview?(bugs) → superreview+
Waiting for the trunk to reopen before I check this in.
Attachment #221115 - Flags: approval-branch-1.8.1?(bugs)
Attachment #221115 - Flags: approval-branch-1.8.1?(bugs) → approval-branch-1.8.1+
Fix checked in to trunk and 1.8 branch.  I have opened bug 337521 on the underlying issue of the template builder regression on the trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: