399800 - Allow modifying the initial left-pane selection when opening Places Organizer

Status: VERIFIED FIXED

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

Description

[Shawn Wilsher :sdwilsh]

Calling PlacesCommandHook.showPlacesOrganizer with a place: URI doesn't load the URI in the organizer like it implies.

Looking into this, I placed some debugging output in selectPlaceURI, and that would get dumped out to the console immediately before this line would dump something:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/places/content/places.js&rev=1.109#113

I'm guessing the second call overrides anything passed in?

Comment 1

[Shawn Wilsher :sdwilsh]

More debugging output:

*** selectPlaceURI('place:type=0&sort=4&maxResults=10');
*** Calling onPlaceSelected
*** Node URI: place:folder=2&queryType=1

selectPlaceURI is the global function in places.js

Comment 2

[Dietrich Ayala (:dietrich)]

Attachment: fix

use the new location setter instead of the global func. remove the global func since it has no other callers.

Comment 3

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

trying to wrap my head around this:

1)

  C:\builds\trunk\mozilla\browser\base\content\browser-places.js(332):        organizer.selectPlaceURI(aPlace);

doesn't this one call the global function you are removing?

2)

  C:\builds\trunk\mozilla\browser\components\places\content\places.js(49):  PlacesOrganizer._places.selectPlaceURI(placeURI);

once you remove that one, who is calling selectPlaceURI() in tree.xml?

(does using the location setter do the same thing as selectPlaceURI in tree.xml?)

  C:\builds\trunk\mozilla\browser\components\places\content\tree.xml(168):      <method name="selectPlaceURI">

Comment 4

[Dietrich Ayala (:dietrich)]

Attachment: fix v2

> 1)
> 
>   C:\builds\trunk\mozilla\browser\base\content\browser-places.js(332):       
> organizer.selectPlaceURI(aPlace);
> 
> doesn't this one call the global function you are removing?

the previous patch was completely bogus. new patch which actually works.

Comment 5

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

Comment on attachment 287866 [details] [diff] [review]
fix v2

how come we don't need the call to this.onContentTreeSelect() anymore?

it looks like we may still want to call that.

Comment 6

[Dietrich Ayala (:dietrich)]

Attachment: fix v3

Yes, still need that, added back in to onPlaceSelected, which seems more appropriate as that's where the content pane is loaded from.

Comment 7

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

dietrich, if while fixing this, you wanted to rename the places tree binding method or the function in utils.js (so that both weren't named "selectPlaceURI") that would be a big help.

http://lxr.mozilla.org/seamonkey/search?string=selectPlaceURI

Comment 8

[Asaf Romano (gone)]

Fixed in bug 404884.

Comment 9

[Tracy Walker [:tracy]]

Shawn, please verify this bug, thanks.

Comment 10

[Shawn Wilsher :sdwilsh]

I nuked the code that I was using in my tree that would test this and don't expect to have time to recreate it any time soon.

Comment 11

[Tracy Walker [:tracy]]

ok, verified per check-in then

Comment 12

[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