Closed Bug 268553 Opened 15 years ago Closed 15 years ago

Can't add a new folder on the bookmarks toolbar after having deleted a folder

Categories

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

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
Firefox1.5

People

(Reporter: michali.sarris, Assigned: vlad)

Details

(Keywords: fixed1.8, Whiteboard: [asaP1])

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

If i delete a folder from the bookmarks toolbar i'm not able to add a new folder
untill restarting firefox.

Reproducible: Always
Steps to Reproduce:
1. Add a folder to the bookmarks toolbar by rightclicking it and choosing "New
Folder...".
2. Delete the just created folder.
3. Try to add another new folder the same way as point 1.

Actual Results:  
I hear an alert beep and nothing happens.

Expected Results:  
A dialog should have appeared to add a new folder.
I confirm (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5)
Gecko/20041107 Firefox/1.0)
confirmed with Windows branch build 2004-11-07-07-0.11
Status: UNCONFIRMED → NEW
Ever confirmed: true
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

Extension of this bug?

Reproducible: Always
Steps to Reproduce:
1. Right Click on Bookmarks Toolbar
2. Manage Folder
3. Click New Folder
4. Creates New Folder
5. Drag and Drop links into this Folder
6. Click New Folder
7. Creates New Folder in Bookmarks but not in Bookmarks Toolbar

Actual Results:  
Creates 2nd New Folder in Bookmarks but not Bookmarks Toolbar

Expected Results:  
Add New Folder in Bookmarks Toolbar
I think this is the bug you mean is:
https://bugzilla.mozilla.org/show_bug.cgi?id=268551

I found it the same time as this bug.

(In reply to comment #3)
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107
Firefox/1.0
> 
> Extension of this bug?
> 
> Reproducible: Always
> Steps to Reproduce:
> 1. Right Click on Bookmarks Toolbar
> 2. Manage Folder
> 3. Click New Folder
> 4. Creates New Folder
> 5. Drag and Drop links into this Folder
> 6. Click New Folder
> 7. Creates New Folder in Bookmarks but not in Bookmarks Toolbar
> 
> Actual Results:  
> Creates 2nd New Folder in Bookmarks but not Bookmarks Toolbar
> 
> Expected Results:  
> Add New Folder in Bookmarks Toolbar
> 

Assignee: vladimir → vladimir+bm
Confirmed on Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.5)
Gecko/20041107 Firefox/1.0
still seeing this on 1.0.1 from 02/22

a sort of workaround is to open a new window and add the new folder there.
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Whiteboard: [asaP1]
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050410
Firefox/1.0+

I tried to reproduce this bug. On the current nightly I'm using, I cannot even
create a 'new folder' once straight from the bookmarks toolbar.
WFM
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050422
Firefox/1.0+
(Windows 2000)
WFM, seems to be fixed on the trunk!

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050418
Firefox/1.0+
WFM
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050507
Firefox/1.0+
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Reopening.

I can reproduce this 100% with a clean profile.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050511
Firefox/1.0+
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050512
Firefox/1.0+ ID:2005051212

WFM following steps in comment 0, on both an existing and a new profile.
The problem is that BookmarksMenu._target is not released (unset) after a
bookmark deletion. After, it is passed as argument for
BookmarksCommand.createNewFolder(realTarget); (line 1110)

Adding one line in firefox\chrome\browser.jar\content\browser\bookmarks\bookmarks.js
should fix this


  deleteBookmark: function (aSelection)
  {
    // call checkSelection here to update the immutable and other
    // flags on the selection; when new resources get created,
    // they're temporarily not valid because they're not in a
    // bookmark container.  So, they can't be removed until that's
    // fixed.
    BookmarksUtils.checkSelection(aSelection);
    BookmarksUtils.removeAndCheckSelection("delete", aSelection);
//adding the next line
    BookmarksMenu._target= null;
  },


of course, i don't know if this is the good syntax and if it is in the good file
(maybe in BookmarksUtils.removeAndCheckSelection() from ?)

kind-of-review ?
Ooh, good call.

-> me, unless someone else wants to write a real patch.
Assignee: vladimir+bm → mconnor
Status: REOPENED → NEW
Priority: -- → P3
Target Milestone: --- → Firefox1.1
wfm. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050609
Firefox/1.0+ ID:2005060908. someone needs to mark this resolved.
(In reply to comment #15)
> wfm. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050609
> Firefox/1.0+ ID:2005060908. someone needs to mark this resolved.

No, I can still reproduce this
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050615
Firefox/1.0+ ID:2005061513

Attached patch Patch from comment 13 (obsolete) — Splinter Review
This patch uses the code from comment 13, also added into the cut command as
per comment 17. I have briefly tested this and it doesn't appear to break any
of the functions however as per comment 12 I am unable to reproduce so can't
verify that it fixes the problem.

Could someone test this to see if it fixes this bug.
(In reply to comment #19)
> (From update of attachment 189116 [details] [diff] [review] [edit])
> won't this break the bookmarks manager?

After some testing, it does not break the bookmarks manager, however it does
give the following error in the JS console:

Error: BookmarksMenu is not defined
Source File: chrome://browser/content/bookmarks/bookmarks.js
Line: 408

So a different solution is probably required.
(thanks Mano)
Assignee: mconnor → vladimir
Flags: blocking-aviary1.5+ → blocking1.8b4+
Attached patch patchSplinter Review
Just clear out _target and _selection in destroyContextMenu.  Note that the
patch includes some extra comment cleanup (# isn't a comment character in JS,
grr), but the fix is just two lines near the start.
Attachment #194122 - Flags: review?(bugs.mano)
Can we get this landed on the trunk and verified before taking on the branch?
k, in on trunk.
Status: NEW → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
I am unable to verify this on today's trunk build since I can't create a new
folder by right clicking - I filed Bug 306457 to address this issue. 
still blocking on accessibility regression in order to verify fix on trunk
before branch approval
(In reply to comment #25)
> I am unable to verify this on today's trunk build since I can't create a new
> folder by right clicking - I filed Bug 306457 to address this issue. 

That is only broken now if you have no bookmarks toolbar folder. I'm working on
a fix for that, but it should be good enough to test this fix.
verified on trunk with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20050901 Firefox/1.6a1
Status: RESOLVED → VERIFIED
Attachment #194122 - Flags: approval1.8b4? → approval1.8b4+
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.