331801 - uncaught exception bookmarking about: or chrome: URIs

Status: RESOLVED FIXED

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

Description

[Phil Ringnalda (:philor)]

STR:
1. Open about:config or chrome://global/content/console.xul
2. Add Bookmark

Result:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsINavBookmarksService.getItemTitle]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://browser/content/places/bookmarkProperties.js :: BPP__populateProperties :: line 2707"  data: no]

and empty title and location in the Add Bookmark dialog.

Comment 1

[Joe Hughes]

Attachment: Patch to add exception-handling code (to handle unavailable metadata) when populating bookmark properties

Comment 2

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

Comment on attachment 216767 [details] [diff] [review]
Patch to add exception-handling code (to handle unavailable metadata) when populating bookmark properties

you should probably add to your comment though:

// title not available, e.g. for about: or chrome: URIs.

Comment 3

[Brett Wilson]

I think GetItemTitle should be fixed. This looks like a 1 line change to SetIsVoid(PR_TRUE) and return when there are no results. Then you don't need the ugly JavaScript exception stuff.

Comment 4

[Joe Hughes]

I didn't comment that exception, because strictly speaking, those URIs *do* have titles as shown on the title bar and in the session history--it's just that (if I understand correctly) the history service isn't currently providing those titles, which bug 332255 would change.

Comment 5

[Joe Hughes]

So, Brett, you going to land bug 332254 instead?

Comment 6

[Brett Wilson]

I don't think the history service should provide title for those things. They aren't added to history, therefore there is no title, which I think is correct behavior.

Do you want to do bug 332254? I'm in the middle of a lot of performance stuff.

Comment 7

[Brett Wilson]

Looking at the code for getKeywordForURI, it shouldn't except. Were you getting exceptions from that.

Comment 8

[Joe Hughes]

Attachment: Alternate version that Brett wanted, which changes getItemTitle (and a couple others) to return a void string on no results rather than throwing an exception

Comment 9

[Brett Wilson]

I don't think the folder title/type things should be changed. If they don't exist, then you have a reference to a bad folder ID which is bad and you should pay for it with pain. You should be getting folder IDs from the bookmark/history service and they should be valid, which is fundamentally different than URIs which come from anywhere and anybody can make them up. The rest looks good.

Comment 10

[Joe Hughes]

Attachment: Reverted the behavior of getFolderTitle() but added some documentation about its current behavior

Comment 11

[Brett Wilson]

Comment on attachment 216786 [details] [diff] [review]
Reverted the behavior of getFolderTitle() but added some documentation about its current behavior

Perfect, thanks.

Comment 12

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

Comment on attachment 216786 [details] [diff] [review]
Reverted the behavior of getFolderTitle() but added some documentation about its current behavior

cool. r=[e-mail]

Comment 13

[Joe Hughes]

Landed on branch and trunk.

Comment 14

[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