549491 - "The root node is never visible" exception when details of the root node are modified

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marcia Knous [:marcia]]

Seen while running Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a2) Gecko/20100228 MozillaDeveloperPreview/3.7a2 (.NET CLR 3.5.30729)

I get this filling the error console: 

Error: [Exception... "'The root node is never visible' when calling method: [nsINavHistoryResultViewer::nodeHistoryDetailsChanged]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: file:///C:/Program%20Files%20(x86)/Mozilla%20Developer%20Preview%203.7%20Alpha%202/components/nsLivemarkService.js :: LS_insertLivemarkChild :: line 604"  data: no]
Source File: file:///C:/Program%20Files%20(x86)/Mozilla%20Developer%20Preview%203.7%20Alpha%202/components/nsLivemarkService.js
Line: 604

testing with an existing trunk profile - only addon installed is Weave Sync.

Comment 1

[Marco Bonardo [:mak]]

Regression from bug 520659

Comment 2

[Asaf Romano (gone)]

STR, please.

Comment 3

[Marcia Knous [:marcia]]

Working on STR. I believe I was running one of the Litmus test cases when it happened so I will revisit that test case and see if I can repro it.

Comment 4

[Asaf Romano (gone)]

Actually, never mind.

Comment 5

[Marcia Knous [:marcia]]

STR: Seen while running Alpha 3

1. http://feeds.delicious.com/v2/rss/recent?min=2&count=15
2. Install feed in Unsorted Bookmarks
3. Select Feed and Reload Live Bookmark
4. Receive error in console.

Comment 6

[Asaf Romano (gone)]

Attachment: patch

Due to xpconnect magic, the exceptions is swallowed when this exception is thrown the normal way.  In particular, calling addVisir from the test did get to the exception line, but it didn't get to the console (probably because addVisit itself doesn't throw if updating the view failed).

Thus, I emulated the error by calling node*Changed directly.

Comment 7

[Marco Bonardo [:mak]]

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

>diff -r ba82b6ecb9ce browser/components/places/tests/chrome/test_bug549491.xul

>+      tree.view.QueryInterface(Ci.nsINavHistoryResultObserver)
>+          .nodeHistoryDetailsChanged(rootNode, rootNode.time, rootNode.accessCount);
>+      tree.view.QueryInterface(Ci.nsINavHistoryResultObserver)
>+          .nodeTitleChanged(rootNode, rootNode.title);

I think you could be able to QI just once

Comment 8

[Marco Bonardo [:mak]]

I fixed my comment before pushing.
http://hg.mozilla.org/mozilla-central/rev/ee7eb27ef77b

with a subsequent push I've also fixed possible random failures due to sync removeAllPages calls.

Comment 9

[Asaf Romano (gone)]

Hey, you didn't have to do this (but thanks!). Next time, please ping via email or IRC.

Comment 10

[Marco Bonardo [:mak]]

no problem, it was a trivial change and I was already pushing.