386392 - Drop pre places/places-bookmarks support from browser/

Status: VERIFIED FIXED

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

Description

[Asaf Romano (gone)]

Comment 1

[Asaf Romano (gone)]

Attachment: part 1

To be done separately:
  * cvs removes for browser/components/bookmarks & browser/components/history
  * cvs removes for some files in browser/locales
  * cvs remove the old microsumarries interface file.
  * --enable-place-bookmarks build option removal
  * MOZ_PLACES_BOOKMARKS->MOZ_PLACES in the xpfe search service (gavin:
   what's that about anyway?)

Comment 2

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

wasn't the plan to keep non-places for alice's performance tests?

Comment 3

[Asaf Romano (gone)]

* Do we have a tinderbox building non-places/non-places-bookmarks?

* Which performance tests? Ts/Txul/Tp regressions are filed as we break them on the default configuration.

Maintaining three different build modes for browser/ does not make a good balance here IMO, esp. as in-browser window UI based on places is hooked up.

Either way, this has to be done before l10n freeze.

Comment 4

[Mike Connor [:mconnor]]

I'm not sure the perf benchmarking on/off is going to be particularly useful, especially if it means we need to maintain other features against the old code.  We should get talos benchmarks for on/off now, and any deltas need to be made up before ship.

If we're at least on par for perf in the general case, or have clearly identified cases where we need to vastly improve perf, I think we're ready to purge the old code.

Comment 5

[Benjamin Smedberg]

Note that you'll probably have to do the on/off benchmarks with static build instead of libxul builds, because the non-places code doesn't build with libxul.

Comment 6

[Dietrich Ayala (:dietrich)]

(In reply to comment #4)
>  We should get talos benchmarks for on/off now, and any deltas need to be made
> up before ship.

This is done. Last week Alice did 20 runs w/ each of the 3 configurations (no places, places, places bookmarks). Old bookmarks can go away now.

Comment 7

[Asaf Romano (gone)]

Attachment: part 1, updated to tip

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 271817 [details] [diff] [review]
part 1, updated to tip

>Index: browser/base/content/browser.xul
>-#include browser-gotreehack.inc

We can remove this file too now, right? Both references to it are being removed.

>Index: browser/components/Makefile.in
>Index: browser/components/build/Makefile.in

Indentation looks wrong in these files.

Also don't forget to remove nsIMicrosummaryServiceNonPlaces.

grep -r --exclude=".#*" --exclude="*.orig" --exclude="*.rej" "MOZ_PLACES" browser/ tells me that you forgot a few in places/content, need to fix before landing.

Want to file a bug about making --disable-places fail when building Firefox? (or do it here?). We should also post to the newsgroups/devnews about this.

We need to just remove the ifdefs in xpfe/components/search, we shouldn't build that directory anymore, so they aren't needed. Can file a new bug for that.

Comment 9

[Asaf Romano (gone)]

Attachment: don't confuse them

Comment 10

[Asaf Romano (gone)]

Attachment: confuse them

toolkit/ patch coming.

Comment 11

[Asaf Romano (gone)]

Attachment: toolkit changes

Comment 12

[Asaf Romano (gone)]

Attachment: for checkin

Comment 13

[Asaf Romano (gone)]

mozilla/configure.in 1.1850
mozilla/browser/build.mk 1.5
mozilla/browser/confvars.sh 1.6
mozilla/browser/base/content/browser-doctype.inc 1.12
mozilla/browser/base/content/browser-gotreehack.inc delete
mozilla/browser/base/content/browser-menubar.inc 1.116
mozilla/browser/base/content/browser-sets.inc 1.99
mozilla/browser/base/content/browser.js 1.815
mozilla/browser/base/content/browser.xul 1.350
mozilla/browser/base/content/browserMountPoints.inc 1.5
mozilla/browser/base/content/global-scripts.inc 1.14
mozilla/browser/base/content/macBrowserOverlay.xul 1.16
mozilla/browser/base/content/nsContextMenu.js 1.18
mozilla/browser/base/content/web-panels.xul 1.21
mozilla/browser/components/Makefile.in 1.60
mozilla/browser/components/nsBrowserGlue.js 1.32
mozilla/browser/components/build/Makefile.in 1.57
mozilla/browser/components/build/nsModule.cpp 1.51
mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp 1.18
mozilla/browser/components/feeds/src/FeedConverter.js 1.26
mozilla/browser/components/feeds/src/FeedWriter.js 1.43
mozilla/browser/components/microsummaries/public/Makefile.in 1.3;
mozilla/browser/components/microsummaries/public/nsIMicrosummaryServiceNonPlaces.idl delete
mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js 1.67
mozilla/browser/components/migration/src/Makefile.in 1.34
mozilla/browser/components/migration/src/nsBrowserProfileMigratorUtils.cpp 1.25;
mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp 1.62
mozilla/browser/components/migration/src/nsIEProfileMigrator.h 1.13
mozilla/browser/components/migration/src/nsOperaProfileMigrator.cpp 1.67
mozilla/browser/components/migration/src/nsOperaProfileMigrator.h 1.20
mozilla/browser/components/migration/src/nsSafariProfileMigrator.cpp 1.42
mozilla/browser/components/migration/src/nsSafariProfileMigrator.h 1.19
mozilla/browser/components/places/Makefile.in 1.8
mozilla/browser/components/places/content/controller.js 1.170
mozilla/browser/components/places/content/history-panel.js 1.20
mozilla/browser/components/places/content/history-panel.xul 1.11
mozilla/browser/components/places/content/placesOverlay.xul 1.14
mozilla/browser/components/places/tests/Makefile.in 1.9
mozilla/browser/components/preferences/main.js 1.9
mozilla/browser/components/search/content/engineManager.js 1.17
mozilla/browser/components/sidebar/src/nsSidebar.js 1.29
mozilla/browser/locales/jar.mn 1.68
mozilla/browser/themes/pinstripe/browser/browser.css 1.56
mozilla/browser/themes/pinstripe/browser/jar.mn 1.44
mozilla/browser/themes/winstripe/browser/browser.css 1.66
mozilla/browser/themes/winstripe/browser/jar.mn 1.39
mozilla/toolkit/components/places/src/Makefile.in 1.26
mozilla/toolkit/components/places/src/nsNavBookmarks.cpp 1.109
mozilla/toolkit/components/places/tests/Makefile.in 1.2

Comment 14

[pal-moz]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071717 Minefield/3.0a7pre ID:2007071717

bookmarks in sidebar is broken. (cannot display them in sidebar.)
caused by this check-in ?

Comment 15

[Phil Ringnalda (:philor)]

Attachment: two more

so we can have a bookmarks sidebar, and pick bookmarks as home pages

Comment 16

[Phil Ringnalda (:philor)]

browser/components/places/jar.mn 1.37
browser/components/preferences/jar.mn 1.18

Comment 17

[Asaf Romano (gone)]

Thanks Phil!

Comment 18

[Phil Ringnalda (:philor)]

Attachment: rm components/bookmarks/

Near as I can tell, this covers all of components/bookmarks/'s few remaining deadcode friends, too.

Comment 19

[Phil Ringnalda (:philor)]

At the very least, the unused strings part of this ought to be blocking whatever turns out to be the string freeze milestone.

Comment 20

[Asaf Romano (gone)]

Comment on attachment 284815 [details] [diff] [review]
rm components/bookmarks/

r=mano excluding the browser/themes files. I would like to remove those only once we move over all the images used wisely by places' jar.mns...

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 284815 [details] [diff] [review]
rm components/bookmarks/

this can wait until after beta,

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Not a blocker, either, though it's got approval to land :)

Comment 23

[Phil Ringnalda (:philor)]

browser/components/bookmarks/.cvsignore delete
browser/components/bookmarks/jar.mn delete
browser/components/bookmarks/Makefile.in delete
browser/components/bookmarks/content/addBookmark2.js delete
browser/components/bookmarks/content/addBookmark2.xul delete
browser/components/bookmarks/content/addBookmark.xul delete
browser/components/bookmarks/content/addBookmark.js delete
browser/components/bookmarks/content/bookmarks.css delete
browser/components/bookmarks/content/bookmarks.js delete
browser/components/bookmarks/content/bookmarksManager.js delete
browser/components/bookmarks/content/bookmarksManager.xul delete
browser/components/bookmarks/content/bookmarksMenu.js delete
browser/components/bookmarks/content/bookmarksPanel.js delete
browser/components/bookmarks/content/bookmarksPanel.xul delete
browser/components/bookmarks/content/bookmarksProperties.js delete
browser/components/bookmarks/content/bookmarksProperties.xul delete
browser/components/bookmarks/content/bookmarksTree.xml delete
browser/components/bookmarks/content/microsummaryPicker.js delete
browser/components/bookmarks/content/selectBookmark.js delete
browser/components/bookmarks/content/selectBookmark.xul delete
browser/components/bookmarks/public/.cvsignore delete
browser/components/bookmarks/public/Makefile.in delete
browser/components/bookmarks/public/nsIBookmarksService.idl delete
browser/components/bookmarks/public/nsIBookmarkTransactionManager.idl delete
browser/components/bookmarks/src/.cvsignore delete
browser/components/bookmarks/src/Makefile.in delete
browser/components/bookmarks/src/nsBookmarksFeedHandler.cpp delete
browser/components/bookmarks/src/nsBookmarksService.cpp delete
browser/components/bookmarks/src/nsBookmarksService.h delete
browser/components/bookmarks/src/nsBookmarkTransactionManager.js delete
browser/components/bookmarks/src/nsForwardProxyDataSource.cpp delete
browser/components/bookmarks/src/nsForwardProxyDataSource.h delete
browser/locales/en-US/chrome/browser/bookmarks/addBookmark.dtd delete
browser/locales/en-US/chrome/browser/bookmarks/bookmarks.dtd delete
browser/locales/en-US/chrome/browser/bookmarks/bookmarks.properties delete
browser/locales/en-US/chrome/browser/bookmarks/bookmarksProperties.dtd delete

Comment 24

[Nick Thomas [:nthomas] (UTC+12)]

philor, you scrub the removed Makefiles out of mozilla/browser/makefiles.sh, right ?

Comment 25

[Phil Ringnalda (:philor)]

If by that you mean "you will scrub ... once I remind you?" then "yes" :)

browser/makefiles.sh 1.4

Comment 26

[Phil Ringnalda (:philor)]

Attachment: rm components/history/

Considerably easier than bookmarks - places still uses the same .dtd, and the .properties lives in toolkit, so only the .js and .xul are dead.

Comment 27

[Asaf Romano (gone)]

Comment on attachment 289327 [details] [diff] [review]
rm components/history/

r=mano

Comment 28

[Phil Ringnalda (:philor)]

browser/components/history/.cvsignore delete
browser/components/history/jar.mn delete
browser/components/history/Makefile.in delete
browser/components/history/content/history.js delete
browser/components/history/content/history-panel.xul delete
browser/makefiles.sh 1.5

Comment 29

[Phil Ringnalda (:philor)]

The theme files wound up picking up bugs of their own (bug 382095, bug 424613, possibly even more I've forgotten about), so in hindsight this has been fixed for months.

Comment 30

[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