372051 - Error: this._ans.setAnnotation is not a function when moving a livemark container

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Asaf Romano (gone)]

There's no generic setAnnotation method.

Comment 1

[Robert Sayre]

what is the test case?

Comment 2

[Robert Sayre]

I am pretty surprised the existing tests didn't hit this

Comment 3

[tabmix.onemen]

just use setAnnotationString since feedURIString and siteURIString are always strings

Comment 4

[tabmix.onemen]

one more thing in LivemarkService.prototype.onContainerMoved

-      siteURIString = this._ans.GetAnnotationString(oldURI, LMANNO_SITEURI);
+      siteURIString = this._ans.getAnnotationString(oldURI, LMANNO_SITEURI);

Comment 5

[tabmix.onemen]

Attachment: patch

Comment 6

[Asaf Romano (gone)]

Comment on attachment 261032 [details] [diff] [review]
patch

patching file toolkit/components/places/src/nsLivemarkService.js
patch unexpectedly ends in middle of line
Hunk #1 FAILED at 417.
1 out of 1 hunk FAILED -- saving rejects to file toolkit/components/places/src/nsLivemarkService.js.rej

We also need to extend test_livemarks.js to test  livemark-movement.

Comment 7

[tabmix.onemen]

Attachment: patch

fix the error in the previous file (hopefully)

Comment 8

[tabmix.onemen]

Attachment: patch

fix the error in the previous file (hopefully)

Comment 9

[Asaf Romano (gone)]

Comment on attachment 261120 [details] [diff] [review]
patch

I still get a failed hunk here only... How did you generate this patch? is your tree up-to-date?

Comment 10

[tabmix.onemen]

can you make new patch from this ?

  onContainerMoved:
  function LS_onContainerMoved(container, newFolder, newIndex) {
    var index = this._getLivemarkIndex(container);
    
    // Update the annotation that maps the folder URI to the livemark feed URI
    var newURI = this._bms.getFolderURI(container);
    var oldURI = this._livemarks[index].folderURI;
    var feedURIString = this._ans.getAnnotationString(oldURI, LMANNO_FEEDURI);
    this._ans.removeAnnotation(oldURI, LMANNO_FEEDURI);
-   this._ans.setAnnotation(newURI, LMANNO_FEEDURI, feedURIString, 0, 
-                           this._ans.EXPIRE_NEVER);
+    this._ans.setAnnotationString(newURI, LMANNO_FEEDURI, feedURIString, 0, 
+                           this._ans.EXPIRE_NEVER);
    
    // Update the annotation that maps the folder URI to the livemark site URI
    var siteURIString;
    try {
-     siteURIString = this._ans.GetAnnotationString(oldURI, LMANNO_SITEURI);
+     siteURIString = this._ans.getAnnotationString(oldURI, LMANNO_SITEURI);
    }
    catch (ex) {
      // will throw if no annotation
    }

    if (siteURIString) {
      this._ans.removeAnnotation(oldURI, LMANNO_SITEURI);
-     this._ans.setAnnotation(newURI, LMANNO_SITEURI, siteURIString, 0,
-                             this._ans.EXPIRE_NEVER);
+     this._ans.setAnnotationString(newURI, LMANNO_SITEURI, siteURIString, 0,
+                             this._ans.EXPIRE_NEVER);
    }
  },

Comment 11

[Asaf Romano (gone)]

[e-mail]: for future, patches are generated using the |cvs diff| utility (or diff, if you're working with a non-cvs tree), you should never to do this manually.

Comment 12

[tabmix.onemen]

Attachment: patch

this patch was generate with wincvs with up-to-date tree

i hope this will work

Comment 13

[Asaf Romano (gone)]

Comment on attachment 261563 [details] [diff] [review]
patch

r=mano.

Comment 14

[tabmix.onemen]

[e-mail]:can you check-in this patch?

Comment 16

[(not reading, please use [e-mail] instead)]

fix checked in.  for bug #380090 I had the same patch, so adding in a r=sspitzer (on top of r=mano).

Checking in nsLivemarkService.js;
/cvsroot/mozilla/toolkit/components/places/src/nsLivemarkService.js,v  <--  nsLivemarkService.js
new revision: 1.11; previous revision: 1.10
done

thanks onemen.one for the patch!

Comment 17

[(not reading, please use [e-mail] instead)]

for simple steps to reproduce:

1)  create a new profile
2)  create a folder in the organize bookmarks dialog named "test"
3)  attempt to dnd the "Latest Headlines" livemark into the "test" folder

you'll get the js errors:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "this._ans.setAnnotation is not a function" {file: "file:///Users/sspitzer/Desktop/trunk-no-places/debug-build/dist/MinefieldDebug.app/Contents/MacOS/components/nsLivemarkService.js" line: 1094}]' when calling method: [nsILivemarkService::onContainerMoved]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://browser/content/places/controller.js :: PMFT_doTransaction :: line 1737"  data: yes]
************************************************************
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file /Users/sspitzer/Desktop/trunk-no-places/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp, line 1571
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "this._ans.setAnnotation is not a function" {file: "file:///Users/sspitzer/Desktop/trunk-no-places/debug-build/dist/MinefieldDebug.app/Contents/MacOS/components/nsLivemarkService.js" line: 1094}]' when calling method: [nsILivemarkService::onContainerMoved]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://browser/content/places/controller.js :: PMFT_doTransaction :: line 1737"  data: yes]
************************************************************

additionally, later you may get problems when trying to open the "test" folder.  something along the lines of:


************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'Illegal value' when calling method: [nsILivemarkService::getSiteURI]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://browser/content/places/menu.xml :: _rebuild :: line 193"  data: no]
************************************************************

(but this might depend on other changes to menu.xml which have not landed yet.)

Comment 18

[Gervase Markham [:gerv]]

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