Closed
Bug 234116
Opened 20 years ago
Closed 20 years ago
Context menu commands don't work in *subfolders* of bookmarks menu
Categories
(Firefox :: Bookmarks & History, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: p_ch)
References
()
Details
(Keywords: regression)
Reported by "powder" at http://forums.mozillazine.org/viewtopic.php?p=373419#373419. Steps to reproduce: 1. Go to a subfolder of the bookmarks menu. 2. Right-click an item. 3. Select "Properties" or "Delete". Result: The bookmarks menu goes away, but the command is not carried out. This appears on my JS console: Error: [Exception... "Node was not found" code: "8" nsresult: "0x80530008 (NS_ERROR_DOM_NOT_FOUND_ERR)" location: "chrome://browser/content/bookmarks/bookmarksMenu.js Line: 32"] Source File: chrome://browser/content/bookmarks/bookmarksMenu.js Line: 32 Regression window (testing trunk builds of Firefox on WinXP): works: 02/03 broken: 02/05 scragz broken: 02/12
Assignee | ||
Comment 1•20 years ago
|
||
This'll be the day that I die...
Comment 2•20 years ago
|
||
Happens on Linux too for me with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7a) Gecko/20040214 Firebird/0.8.0+
Updated•20 years ago
|
OS: Windows XP → All
Reporter | ||
Comment 3•20 years ago
|
||
Bug 20022's patch was backed out yesterday, but I still see this bug, so I guess this bug wasn't caused by that patch. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040215 Firebird/0.8.0+
Comment 4•20 years ago
|
||
Works in 20040204 broken in 20040205 Also bug 210910 was broken again too at the same date.
also broken in latest: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040216 Firefox/0.8.0+
Bug 197427 is one possible candidate for the cause of this problem since it altered removeChild() on the day of the regression (as reported by Hussam). Maybe someone can try backing out the patch and seeing if the problem goes away?
Comment 7•20 years ago
|
||
That line number has to be bogus. Line 32 is in the license for bookmarksMenu.js
Reporter | ||
Comment 8•20 years ago
|
||
If I click the link in the JS console, it highlights the line
"aTarget.removeChild(nodeList[0]);". Context:
/////////////////////////////////////////////////////////////////////////////
// hides the 'Open in Tabs' on popuphidden so that we won't duplicate it -->
hideOpenInTabsMenuItem: function (aTarget)
{
var nodeList;
nodeList = aTarget.getElementsByAttribute("class", "openintabs-menuitem");
if (nodeList.length > 0)
> aTarget.removeChild(nodeList[0]);
nodeList = aTarget.getElementsByAttribute("class", "openintabs-menuseparator");
if (nodeList.length > 0)
aTarget.removeChild(nodeList[0]);
},
Comment 9•20 years ago
|
||
Whose bright idea was it to use # characters to comment out the license? In JS files, that's usually /*...*/ commenting. Thanks, Jesse, for the context.
Comment 10•20 years ago
|
||
> That line number has to be bogus. Line 32 is in the license for > bookmarksMenu.js All hail the mighty preprocessor, for it makes line numbers in chrome useless. Of course it could save the line number info and that could be passed to the JS engine to provide useful line numbers, but.... As for that exception, here is what's going on: 1) GetElementsByAttribute will return not only children but also other descendants 2) Clearly the thing it's returning in this case is NOT a child 3) The old XUL removeChild used to just silently ignore you when you asked it to remove a random node that was not a child. After the changes in bug 197427, it properly throws an exception. 4) I have no idea whether you want to just catch the exception or whether this code wants to be doing something a little smarter in cases when the first thing it gets is NOT a child (like actually looking for a child in that list or something.... since the search is depth-first, the first node in the list could be not a child even if there are children in the list). Chances are, this code was buggy in some subtle way anyway due to items 1 and 4 above, so it's a good thing we fixed bug 197427. ;)
Updated•20 years ago
|
Reporter | ||
Comment 11•20 years ago
|
||
Bug 234717, "Right-clicking a File Within a Bookmarks Folder in the Bookmarks Menu Makes That Folder Inaccessible if Nothing is Selected on the Context Menu", has the same error message and is probably the same bug.
Updated•20 years ago
|
Flags: blocking0.9?
Comment 12•20 years ago
|
||
Strange, works for me for a bookmarks being shown in the bookmark toolbar, and in folders (and in those folders, ad infinitum) being shown in the bookmark toolbar. HOWEVEVER, when I start from the Bookmarks menu, then the context commands fail (even for bookmarks in the personal toolbar folder.) Note Using: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7a) Gecko/20040219 Firefox/0.8.0+ Can anyone else confirm my observations?
Comment 13•20 years ago
|
||
Yes, same for me - personal toolbar bookmarks context menu works, but not on main menu bookmarks.
Comment 14•20 years ago
|
||
(In reply to comment #10) > Chances are, this code was buggy in some subtle way anyway due to items 1 and 4 > above, so it's a good thing we fixed bug 197427. ;) I would say the code isn't buggy. It's just that for some reason the code isn't being called for each sub menu (folder), see bug 234717. There's a patch to work around this attached to that bug.
(In reply to comment #13) > Yes, same for me - personal toolbar bookmarks context menu works, but not on > main menu bookmarks. Correct. Context menus work on the Bookmarks Toolbar and in the Bookmarks Manager but not the Bookmarks Menu. Interestingly enough, no JavaScript errors are thrown when the dialog fails to appear from a right-click on the Bookmarks Menu. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040229 Firefox/0.8.0+ (daihard: XFT+GTK2; opt. for P4/SSE-2)
Comment 16•20 years ago
|
||
I believe this is the same as "my" bug 23380
Reporter | ||
Comment 17•20 years ago
|
||
*** Bug 233380 has been marked as a duplicate of this bug. ***
Comment 18•20 years ago
|
||
*** Bug 176880 has been marked as a duplicate of this bug. ***
Comment 19•20 years ago
|
||
The proposed fix for Bug 234116 will also fix this bug too.
Comment 20•20 years ago
|
||
(In reply to comment #19) > The proposed fix for Bug 234116 will also fix this bug too. Sorry for the bug spam. I meant "The proposed fix for Bug 234717 will also fix this bug too" Again, sorry for the bug spam
Assignee | ||
Comment 21•20 years ago
|
||
fixed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Flags: blocking0.9?
Comment 22•18 years ago
|
||
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•