Assertion when right clicking in the Bookmarks Manager, open in tabs disabled for folders in the left hand pane of the organizer

RESOLVED FIXED in Firefox 3 beta1

Status

()

RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: polidobj, Assigned: dietrich)

Tracking

({assertion, regression})

Trunk
Firefox 3 beta1
assertion, regression
Points:
---
Bug Flags:
blocking-firefox3 +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
I'm not sure if this is the same bug as Bug 342491 or not but I get this assert:
ASSERT: null node
Stack Trace:
0:PU_nodelsFolder(null)
1:PU_getURLsForContainerNode(null)
2:PC_buildContextMenu([object XULElement])
3:buildContextMenu([object XULElement])
4:onpopupshowing([object MouseEvent])

The steps are:
1. Open the Bookmarks Manager.
2. Select a folder in the left pane.  This should result in no items being selected in the right pane.  
3. Right click in the right pane where there is no item.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007090504 Minefield/3.0a8pre ID:2007090504
Flags: blocking-firefox3?

Comment 1

11 years ago
this also happens on Mac and in the bookmarks sidebar.
Duplicate of this bug: 396980
(Assignee)

Updated

11 years ago
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Firefox 3 M9
(Assignee)

Comment 3

11 years ago
Created attachment 281862 [details] [diff] [review]
fix

also fixes some typos elsewhere
Assignee: nobody → dietrich
Status: NEW → ASSIGNED
Attachment #281862 - Flags: review?(sspitzer)
looks good, r=sspitzer

style nit, up to you:

I prefer:

+      if (!openContainerInTabsItem.hidden && this._view.selectedNode &&
+          PlacesUtils.nodeIsContainer(this._view.selectedNode)) {

instead of:

+      if (!openContainerInTabsItem.hidden && this._view.selectedNode != null &&
+          PlacesUtils.nodeIsContainer(this._view.selectedNode)) {

hmm, those typos were part of the fix for bug #175124 which has been verified.

looking at the typos you found (good catch!), the folder-but-excluding items scenario must be broken.  

let's figure out how to test that, as it seems like you've fixed a bug that we could put into litmus.
Attachment #281862 - Flags: review?(sspitzer) → review+
here's what's broken that you have fixed:

before your fix, we can't right click, "open in tabs" on folders in the left hand pane of the organizer (excluding items = true).

the menu item is disabled because the existing code will throw the following exception:

ReferenceError: node is not defined
Flags: in-litmus?
Summary: Assertion when right clicking in the Bookmarks Manager → Assertion when right clicking in the Bookmarks Manager, open in tabs disabled for folders in the left hand pane of the organizer
Depends on: 175124
Keywords: regression
(Assignee)

Comment 6

11 years ago
Created attachment 281884 [details] [diff] [review]
fix

style nit addressed

requesting approval: fixes this bug and a regression from bug 175124.
Attachment #281862 - Attachment is obsolete: true
Attachment #281884 - Flags: approval1.9?

Updated

11 years ago
Flags: blocking-firefox3? → blocking-firefox3+

Updated

11 years ago
Attachment #281884 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 7

11 years ago
Checking in browser/components/places/content/controller.js;
/cvsroot/mozilla/browser/components/places/content/controller.js,v  <--  controller.js
new revision: 1.181; previous revision: 1.180
done
Checking in browser/components/places/content/utils.js;
/cvsroot/mozilla/browser/components/places/content/utils.js,v  <--  utils.js
new revision: 1.67; previous revision: 1.66
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Verified fixed
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9) Gecko/2008053008 Firefox/3.0

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060309 Firefox/3.0


But, while testing, found and confirmed bug related to "Open in all tabs": bug 437686

Comment 9

10 years ago
don't need to have a test case that explicitly checks for assertions, it's sort of assumed testers will be keeping an eye out for such things in all testing.
Flags: in-litmus? → in-litmus-
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.