360029 - Remove Places icon from personal toolbar

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

Initial goal for Places in Firefox 3 is to build the existing UI components on top of the new backend. This means that the link to the old Places UI is obsolete, and needs to be removed. We'll figure out what/where should link to the bookmarks organizer during Fx3 planning, if different from the bookmarks menu.

Comment 1

[Phil Ringnalda (:philor)]

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

Comment 2

[Phil Ringnalda (:philor)]

Don't forget to kill the "Star (On|Off)" toolbar button while you're in the neighborhood.

Comment 3

[Joey Minta]

Attachment: work in progress

I'm not entirely sure if this is the right bug for this, but it seemed the closest I could find.  This is a work-in-progress to remove the Star and add back View->Sidebar->Bookmarks.  Currently the biggest problem is that I can't get the text-search to work.  Anything I type just blanks the whole tree.  I was having some weird searching problems in the Organize Bookmarks dialog too, though, so I'm not sure if the problem is on mind end or not.  Feedback/help strongly desired. :-)

Comment 4

[Dan Mills [:thunder]]

(In reply to comment #3)

I gave your patch a read, it looks good for the most part (and similar to mine in bug 362292 in spirit).

One thing:  I don't want us to have a different chrome uri for the places sidebar vs the old one.  So I set up browser/components/places/jar.mn to provide the old uri.  So this is not needed:

+#ifndef MOZ_PLACES_BOOKMARKS
                  type="checkbox" group="sidebar" sidebarurl="chrome://browser/content/bookmarks/bookmarksPanel.xul"
+#else
+                 type="checkbox" group="sidebar" sidebarurl="chrome://browser/content/places/bookmarks-panel.xul"
+#endif


The root of the tree we're using is also different.

Otherwise, nothing really pops out to me as wrong.  I'm going to put up a new patch on the sidebar bug which should look a little better soon.

Comment 5

[Dan Mills [:thunder]]

Attachment: Remove places toolbar button

This is included in jminta's patch, but is only the part concerning this bug.

Jminta, would you like to take this bug? or should I?

Comment 6

[Joey Minta]

(In reply to comment #5)
> Jminta, would you like to take this bug? or should I?
> 
Feel free to take it.  I'm heading on vacation for the weekend anyway.
Note: I combined these patches because I didn't think we should remove the Places icon until there was a replacement sidebar ready, otherwise a fair bit of functionality is lost.

Comment 7

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

Comment on attachment 252668 [details] [diff] [review]
Remove places toolbar button

thunder!  sorry for the delay, thanks for being patient.

the code removal looks good, but there are going to be some entities, a js function (togglePlacesPopup()), and some css rules that are no longer needed.

can you double check that they are no longer needed (after you remove this button) and submit a new patch?

Comment 8

[Dan Mills [:thunder]]

Attachment: Remove toolbar button, v2

Comment 9

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

Comment on attachment 253653 [details] [diff] [review]
Remove toolbar button, v2

dan, is this everything?  what about the placePopup.dtd?

Comment 10

[Dan Mills [:thunder]]

Attachment: Remove toolbar button, v3

Oops, I missed the dtd and the properties files.
Here's another patch that removes those.

Comment 11

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

Comment on attachment 253665 [details] [diff] [review]
Remove toolbar button, v3

r=sspitzer

Comment 12

[Phil Ringnalda (:philor)]

I like the poetry of keeping "_placesPopup: null," as the only popup remnant, but is poetry really enough of a reason to keep it?

Comment 13

[Phil Ringnalda (:philor)]

Also, http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/browser.css#1355 and http://mxr.mozilla.org/seamonkey/source/browser/themes/pinstripe/browser/browser.css#1209 which would then leave the fish eating a bubble browser/themes/(win|pin)stripe/browser/places/placesIcon.png unused

Comment 14

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

phil, thanks for listing the extra things that need removing.  

I'll remove them in my local tree before checking in for dan.

Comment 15

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

Attachment: Remove toolbar button, v4

carrying over my own review, removing additional references per phil.

Comment 16

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

fix checked into the trunk.

Checking in base/content/browser-places.js;
/cvsroot/mozilla/browser/base/content/browser-places.js,v  <--  browser-places.j
s
new revision: 1.25; previous revision: 1.24
done
Checking in base/content/browser.css;
/cvsroot/mozilla/browser/base/content/browser.css,v  <--  browser.css
new revision: 1.23; previous revision: 1.22
done
Checking in base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.336; previous revision: 1.335
done
Checking in components/places/jar.mn;
/cvsroot/mozilla/browser/components/places/jar.mn,v  <--  jar.mn
new revision: 1.32; previous revision: 1.31
done
Removing components/places/content/placesPopup.js;
/cvsroot/mozilla/browser/components/places/content/placesPopup.js,v  <--  places
Popup.js
new revision: delete; previous revision: 1.2
done
Removing components/places/content/placesPopup.xul;
/cvsroot/mozilla/browser/components/places/content/placesPopup.xul,v  <--  place
sPopup.xul
new revision: delete; previous revision: 1.5
done
Checking in locales/jar.mn;
/cvsroot/mozilla/browser/locales/jar.mn,v  <--  jar.mn
new revision: 1.63; previous revision: 1.62
done
Removing locales/en-US/chrome/browser/places/placesPopup.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/placesPopup.dtd,v
<--  placesPopup.dtd
new revision: delete; previous revision: 1.1
done
Removing locales/en-US/chrome/browser/places/placesPopup.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/placesPopup.propert
ies,v  <--  placesPopup.properties
new revision: delete; previous revision: 1.1
done
Checking in themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.cs
s
new revision: 1.41; previous revision: 1.40
done
Checking in themes/pinstripe/browser/jar.mn;
/cvsroot/mozilla/browser/themes/pinstripe/browser/jar.mn,v  <--  jar.mn
new revision: 1.39; previous revision: 1.38
done
Checking in themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.cs
s
new revision: 1.54; previous revision: 1.53
done
Checking in themes/winstripe/browser/jar.mn;
/cvsroot/mozilla/browser/themes/winstripe/browser/jar.mn,v  <--  jar.mn
new revision: 1.35; previous revision: 1.34
done

Comment 17

[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