In Firefox 36 we expose constant guid roots, and thus we don't need anymore the roots table. We will wait at least until Firefox 40 to do the removal though.

This requires changing /toolkit/components/places/Database.cpp /toolkit/components/places/nsPlacesTables.h to remove references to moz_bookmarks_roots the only thing that should not be modified is the MigrateV25Up() method.

Hi, I would like to work on this for my first contribution here. I have already checked the files that require changing. If you have any other advice, I'd be glad to listen to them. Thanks

Hi, it shouldn't be too hard. You can have a look at these articles: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build http://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/index.html https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Thank you for the links. I removed all the occurrences excepted the ones in MigrateV25Up method as asked.

Review ping ?

Oh, sorry, I missed this in the middle of bugmails, cause you didn't set the review flag. When attaching a patch, please always set either review or feedback flag to the mentor. When in need of information you can use the "Need more information" flag at the bottom of the bug. Only one flag at a time is needed to simplify tracking. I'll look at this now.

Comment on attachment 8724770 [details] [diff] [review] bug1143712.patch Review of attachment 8724770 [details] [diff] [review]: ----------------------------------------------------------------- ok, there's one missing piece. With these changes we will stop creating the table in new profiles, but we should also remove it from old profiles. To do that, you need to bump DATABASE_SCHEMA_VERSION to 31, and add a MigrateV31Up() method Database.cpp/.h In the method you should execute a "DROP TABLE IF EXISTS moz_bookmarks_roots" then you should also bump CURRENT_SCHEMA_VERSION in toolkit/components/places/tests/head_common.js to 31 Finally, we need to create a new database file named placesV31.sqlite in toolkit\components\places\tests\migration. Ideally to do that one could just launch the build with your patch applied (./mach run after build succeeds with your patch), then copy the generated places.sqlite in the profile folder to placesV31.sqlite. At this point we need a last operation, that requires opening this db (can use Sqlite Manager add-on) and issuing the "VACUUM" query so it gets shrinked from 10MB to about 2MB. But eventually I could take care of the file generation. ::: toolkit/components/places/Database.cpp @@ +272,5 @@ > rv = newRootStmt->BindUTF8StringByName(NS_LITERAL_CSTRING("guid"), > aGuid); > if (NS_FAILED(rv)) return rv; > rv = newRootStmt->Execute(); > if (NS_FAILED(rv)) return rv; you should also remove these bindings and execute, since they are related to newRootStmt that you removed. Otherwise, this won't even compile. If you already have a build you can rebuild using ./mach build binaries

Sorry about the fact that I didn't add the flag on my message, and thank you for all this help. I updated my patch with all the new modifications. I tried to VACUUM the places.sqlite using SQLite manager with SQL query or with the compact database option but it didn't work.

Comment on attachment 8730260 [details] [diff] [review] bug1143712.patch Review of attachment 8730260 [details] [diff] [review]: ----------------------------------------------------------------- ok, but please set the review flag on a reviewer :)

Comment on attachment 8730260 [details] [diff] [review] bug1143712.patch Review of attachment 8730260 [details] [diff] [review]: ----------------------------------------------------------------- Ok, there's only one mistake that is not bumping DATABASE_SCHEMA_VERSION in Database.h... but I can do that I will add the missing db file and push to Try for you.

Thank you Marco for your mentorship and your patience on my first bug fix.

thank you for your time and availability. Feel free to pick up more bugs around, we don't lack them :)