Closed Bug 327036 Opened 16 years ago Closed 16 years ago

OS Bookmarks menu nonfunctional

Categories

(Firefox :: Bookmarks & History, defect, P1)

PowerPC
macOS
defect

Tracking

()

RESOLVED FIXED
Firefox 2 alpha1

People

(Reporter: pamg.bugs, Assigned: annie.sullivan)

Details

(Keywords: fixed1.8.1)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060131 Firefox/1.5

None of the items in the OS's Bookmarks menu, in the app's main menubar, work.  Plain bookmarks don't go anywhere; folders show a hierarchical arrow but don't expand.

Reproducible: Always

Steps to Reproduce:
1. Launch Firefox
2. Open "Bookmarks" menu from app menubar
3. Choose one of the bookmarks or bookmark folders

Actual Results:  
Nothing happens.  If a bookmark is chosen, it blinks and the menu goes away, but that's all.  Hierarchical menus (folders) are completely unresponsive, doing nothing but highlighting on mouseover.

Expected Results:  
Folders expand hierarchically.  Bookmarks go to websites.

This is the 20060131 Places build, posted as the 20060128 nightly.
Assignee: nobody → annie.sullivan
Priority: -- → P1
Target Milestone: --- → Firefox 2 alpha1
As I was working on this bug, I ran into a lot more issues with the menu and toolbar on Mac.  This patch fixes them:
* There shouldn't be context menus on menuitems for mac because they're hard to use and buggy (this is consistent with 1.5)
* Nodes should be wrapped with \n instead of \r\n because the transerable system converts \r\n differently on Mac.
* The _rebuild method of menu should make sure the resultNode is open before checking its childCount.
* Bookmarks in menus should open on the command event, not the click event.  (This is better for accessibility, and also the click event doesn't go through to the bookmarks menu on the menubar).
* XBL bindings need to be manually attached to menupopups that are children of the bookmarks menu on mac because the menubar items aren't real dom nodes and styles don't get updated correctly.
Attachment #212859 - Flags: review?(bugs)
Comment on attachment 212859 [details] [diff] [review]
Lots of fixes for mac bugs

A few nits:

>+#ifdef XP_MACOSX
>+          // On Mac OSX, we need to manually attach the XBL binding for the bookmarks menu,
>+          // because menus in the OSX menubar aren't real DOM nodes, and they don't get styles
>+          // applied.
>+          var isMenubarChild = false;
>+          var currentNode = this.parentNode;
>+          while (currentNode && !isMenubarChild) {
>+            if (currentNode.id == "bookmarksMenu")
>+              isMenubarChild = true;
>+            currentNode = currentNode.parentNode;
>+          }
>+#endif

Consider renaming the variable isMenubarChild to "needsBindingAttachment" or some similar more compact form, since that (at least at this point) expresses what this variable is for, rather than the condition that assigned it. This will make the code easier to decipher later, even if it requires more modification for other purposes. 

>+#ifdef XP_MACOSX
>+                // If this is a child of the bookmarks menubar, we have to manually attach
>+                // its xbl binding, because it's not a dom node and the style rules don't
>+                // get applied correctly.
>+                if (isMenubarChild)

... and update the name here.

>+                  document.addBinding(popup, "chrome://browser/content/places/menu.xml#places-menupopup");

Since none of this code can exist without controller.js anyway, consider defining this as a constant there.

r=ben@mozilla.org
Attachment #212859 - Flags: review?(bugs) → review+
Status: UNCONFIRMED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.