264031 - Adding bookmarks does not work

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Pavel Penaz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041012 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041012 Firefox/0.10

In todays latest tinderbox branch build adding new bookmarks does not work.
Dragging links to the bookmark manager works though.

Reproducible: Always
Steps to Reproduce:
1. Clean install of Firefox with a new profile
2. Go to any web page
3. Go to bookmarks/Bookmark this page
4. Press OK

Actual Results:  
The add bookmark dialog remains on the screen and the page is not added to
bookmarks.

Expected Results:  
The dialog should go away and the page should appear in the bookmark manager.

Comment 1

[Pavel Penaz]

It seems that dragging links to bookmark toolbar does not work as well. It seems
that links can only be dragged to the root folder in the bookmarks manager.
Dragging links to subfolders does not do anything..

Comment 2

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20041012
Firefox/0.10

confirming

->NEW
->regression

Comment 3

[Peter van der Woude [:Peter6]]

patches in /bookmarks/ since last build
bug 248817
bug 252133
bug 256025
bug 262758
bug 263394

Comment 4

[Peter van der Woude [:Peter6]]

JSConsole error when you try to add / create a new bookmark:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIRDFContainer.InsertElementAt]" 
nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://browser/content/bookmarks/bookmarks.js :: anonymous :: line 1662" 
data: no]

Comment 5

[Ryan Polk (Quark)]

This also seems to cause dragged bookmarks to be placed in the wrong spot by one.

Comment 6

[Ryan Polk (Quark)]

Attachment: partial backout

This fixes the Add Bookmark dialog and the off-by-one error with dragging
bookmarks.

Comment 7

[Ryan Polk (Quark)]

Comment on attachment 161873 [details] [diff] [review]
partial backout

Vlad?  I haven't checked the rationale, but this patch fixes the mentioned
problems.

Comment 8

[Tracy Walker [:tracy]]

seen on Mac too.

smoketest blocker

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

On it; I'd rather not back this out completely, because it does fix the bug with
items getting created in the wrong place.  I'm not sure why the same mechanism
isn't working for new items.  Patch soon.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 264031-adding-bookmarks-broken-0.patch

Fix, as best I can tell, without breaking the recent fix -- this is sort of a
band aid, but it's also the least intrusive patch; otherwise, I'd have to
figure out what's incrementing the index incorrectly.  Bookmarks bits that i've
tested with this seem to work for me, but as has been proven, I suck at
testing, soo..

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 161885 [details] [diff] [review]
264031-adding-bookmarks-broken-0.patch

r+a=[e-mail]

Comment 12

[Steffen Wilberg]

"New Bookmark" and "New Live Bookmark" are still broken in the bookmarks
manager. Reopening. The js console shows:

Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIRDFContainer.InsertElementAt]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://browser/content/bookmarks/bookmarks.js :: anonymous :: line 1666" 
data: no]
Source File: chrome://browser/content/bookmarks/bookmarks.js
Line: 1666

Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsITransactionManager.doTransaction]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://browser/content/bookmarks/bookmarks.js :: anonymous :: line 1392" 
data: no]
Source File: chrome://browser/content/bookmarks/bookmarks.js
Line: 1392

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: 264031-adding-bookmarks-part-2.patch

At least I was honest about sucking at testing. :)

This is my final offer!  (Every single New* and drag-type thingy seems to work
for me)

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 161935 [details] [diff] [review]
264031-adding-bookmarks-part-2.patch

a=asa for aviary checkin.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Okay, really honestly fixed this time.  Read my lips, no new bugs!

Comment 16

[Asaf Romano (gone)]

was it fixed on trunk as well?

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

No, the bug never made it to the trunk.

Comment 18

[sairuh (rarely reading bugmail)]

vrfy'd fixed with 2004101309-0.9+ linux bits on fc2.

Comment 19

[Daniel Lindkvist]

I noticed a problem that can be related to this bug:

1. rightclick personal toolbar and create a new folder
2. rightclick the new folder and delete that
3. repeat step 1 -> no new folder can be added
4. rightclick an existing folder and try to create a new folder there, result:

Error: [Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIRDFContainer.InsertElementAt]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://browser/content/bookmarks/bookmarks.js :: anonymous :: line 1677" 
data: no]
Source File: chrome://browser/content/bookmarks/bookmarks.js
Line: 1677

Comment 20

[Peter van der Woude [:Peter6]]

*** Bug 264368 has been marked as a duplicate of this bug. ***

Comment 21

[Tracy Walker [:tracy]]

Daniel, please search for a dupe and/or file a new bug for what you're seeing
with new folders.  Thanks. 

Comment 22

[Hussam Al-Tayeb]

(In reply to comment #19)
> I noticed a problem that can be related to this bug:
> 
> 1. rightclick personal toolbar and create a new folder
> 2. rightclick the new folder and delete that
> 3. repeat step 1 -> no new folder can be added
> 4. rightclick an existing folder and try to create a new folder there, result:

I see it too. You might want to report this in a new bug.

Comment 23

[Daniel Lindkvist]

Ok submitted bug 264385

Comment 24

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change