Reorganize pre-populated smart bookmarks (add versioning)

VERIFIED FIXED in Firefox 3

Status

()

Firefox
Bookmarks & History
P2
normal
VERIFIED FIXED
10 years ago
7 years ago

People

(Reporter: faaborg, Assigned: mak)

Tracking

Trunk
Firefox 3
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 6 obsolete attachments)

(Reporter)

Description

10 years ago
Beltzner writes in email:

>what I'm thinking is:
>
>- put "Most Visited" on the Bookmark Toolbar as default
>- put "Recently Tagged", and "Recently Bookmarked" as defaults in
>the Bookmarks Menu

We are worried that the current mouse path to these subfolders is too tedious to make them actually useful.  Also, they are not currently very discoverable.
(Reporter)

Comment 1

10 years ago
Requested blocking at least get a wanted, if not blocking.
Flags: blocking-firefox3?
Is there a dupe on this? I thought I'd filed ...
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Assignee: nobody → mak77
Whiteboard: dupeme
Target Milestone: --- → Firefox 3 beta5
Target Milestone: Firefox 3 beta5 → Firefox 3
(Assignee)

Updated

10 years ago
Whiteboard: dupeme → [swag: 0.5d]
(Assignee)

Comment 3

9 years ago
i'm going add versioning to queries.

We cannot remove Smart Bookmarks folder for old users for some reason:
1. We have not annotated it, so we should rely on a folder name
2. A user could have renamed it
3. A user could have added other queries/things inside it

So i'll simply recreate queries in the right place, beta users will have to manually delete their old Smart bookmarks folder

Other problems:
4. a user deletes one query, on version bump we should not recreate it
5. a user moves one query, on version bump we should recreate in the current folder
Status: NEW → ASSIGNED
(In reply to comment #3)
> So i'll simply recreate queries in the right place, beta users will have to
> manually delete their old Smart bookmarks folder

David, one more thing which should be mentioned on the beta user page?
Blocks: 426340
(Assignee)

Updated

9 years ago
Whiteboard: [swag: 0.5d] → [swag: 0.5d][possible TS regression]
(Assignee)

Updated

9 years ago
Summary: Reorganize pre-populated smart bookmarks → Reorganize pre-populated smart bookmarks (add versioning)
Whiteboard: [swag: 0.5d][possible TS regression] → [swag: 0.5d][check for possible TS regression]
(Assignee)

Comment 5

9 years ago
Created attachment 313090 [details] [diff] [review]
patch

reorganized smart bookmarks like requested, i still need to know exact position where we want them (will attach a screen)

notes:
- old smart bookmarks folder will last, user have to delete it manually
- added versioning, so in future we can replace them / add new ones and bump up version
- users can reset their smart bookmarks setting version = 0 (position will not be restored though, only title and uri, and missing ones will be recreated)
- in case of an upgrade, missing smart bookmarks will be recreated, while others will be replaced in the same position (so if a user has moved them they will not bounce to another place)
(Assignee)

Comment 6

9 years ago
Created attachment 313092 [details]
screenshot - looking for position

Mike can you tell me where do we want them in the menu?

For toolbar i suppose that first position (like corrent smart bookmarks folder) is good
Attachment #313092 - Flags: ui-review?(beltzner)
(Assignee)

Updated

9 years ago
Whiteboard: [swag: 0.5d][check for possible TS regression] → [has patch][check for possible TS regression]
(Assignee)

Updated

9 years ago
Attachment #313092 - Attachment is patch: false
Attachment #313092 - Attachment mime type: text/plain → image/png
(Assignee)

Comment 7

9 years ago
Created attachment 313105 [details] [diff] [review]
much better

much cleaner patch, easy management.

While waiting for Beltzner suggestion on position of items, we can go on with review.
Attachment #313090 - Attachment is obsolete: true
Attachment #313105 - Flags: review?(dietrich)

Comment 8

9 years ago
Marco, can you not have an icon for this instead. IMO it has a better name, but still looks ugly?

Comment 9

9 years ago
Typo:
prefBranch.setBoolPref("browser.places.smartBookmarksVersion", -1);
in BrowserGlue.js BrowserGlue.prototype
(Assignee)

Comment 10

9 years ago
(In reply to comment #9)
> Typo:
> prefBranch.setBoolPref("browser.places.smartBookmarksVersion", -1);

thank you, should be Int
Comment on attachment 313105 [details] [diff] [review]
much better


>+        // MOST VISITED
>+        var smart = {queryId: "MostVisited", // don't change this
>+                     itemId: null,
>+                     title: this._placesBundle.GetStringFromName("mostVisitedTitle"),
>+                     uri: this._uri("place:queryType=" +
>+                                    Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY +
>+                                    "&sort=" +
>+                                    Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
>+                                    "&maxResults=" + MAX_RESULTS),
>+                     parent: bmsvc.toolbarFolder,
>+                     position: 0};
>+        smartBookmarks.push(smart);
>+
>+        // RECENLTY BOOKMARKED

typo: RECENTLY

>Index: browser/components/places/tests/unit/test_bookmarks_html.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js,v
>retrieving revision 1.35
>diff -u -8 -p -r1.35 test_bookmarks_html.js
>--- browser/components/places/tests/unit/test_bookmarks_html.js	1 Apr 2008 18:26:09 -0000	1.35
>+++ browser/components/places/tests/unit/test_bookmarks_html.js	2 Apr 2008 16:42:12 -0000
>@@ -85,17 +85,17 @@ const LAST_CHARSET_ANNO = "URIProperties
> 
> // main
> function run_test() {
>   // get places import/export service
>   var importer = Cc["@mozilla.org/browser/places/import-export-service;1"].getService(Ci.nsIPlacesImportExportService);
> 
>   // avoid creating the places smart folder during tests
>   Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch).
>-  setBoolPref("browser.places.createdSmartBookmarks", true);
>+  setIntPref("browser.places.smartBookmarksVersion", 9999);

ew. can you please implement the pref so that -1 = rebuild, 0 = disabled, and used the disabled value in the tests?

also, please add a test that confirms the queries are created properly, as well as the prefs set properly.

finally, update the comment here: http://mxr.mozilla.org/seamonkey/source/browser/components/nsIBrowserGlue.idl#73
Attachment #313105 - Flags: review?(dietrich) → review-
(Assignee)

Comment 12

9 years ago
ok, so after chat, modifying to
-1: disable creation for users that don't want them
0: restore to current version
(Assignee)

Updated

9 years ago
Whiteboard: [has patch][check for possible TS regression] → [has patch][needs UI-R beltzner]
(Assignee)

Comment 13

9 years ago
Created attachment 313590 [details] [diff] [review]
patch v2, with unit test

implements new behaviour, plus dedicated unit test
Attachment #313105 - Attachment is obsolete: true
Attachment #313590 - Flags: review?(dietrich)
(Assignee)

Comment 14

9 years ago
PS: while working on unit test i found a typo in bookmarks idl... i think it does not deserve having a dedicated bug report so i've fixed it here:

-  void moveItem(in long long aFolder, in long long newParent, in long aIndex);
+  void moveItem(in long long aItemId, in long long newParent, in long aIndex);
(Assignee)

Comment 15

9 years ago
Created attachment 313591 [details] [diff] [review]
patch v2.1

humm rethinking to that since i've another bug open on moveItem i'm moving that fix there, it's more pertinent. Removed
Attachment #313590 - Attachment is obsolete: true
Attachment #313591 - Flags: review?(dietrich)
Attachment #313590 - Flags: review?(dietrich)
Comment on attachment 313591 [details] [diff] [review]
patch v2.1



>+        var smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
>+        // set current itemId, parent and position if Smart Bookmark exists
>+        for each(var itemId in smartBookmarkItemIds) {
>+          var queryId = annosvc.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
>+          for (var i = 0; i < smartBookmarks.length; i++){
>+            if (smartBookmarks[i].queryId == queryId) {
>+              smartBookmarks[i].itemId = itemId;
>+              smartBookmarks[i].parent = bmsvc.getFolderIdForItem(itemId);
>+              smartBookmarks[i].position = bmsvc.getItemIndex(itemId);
>+              // remove current item, since it will be replaced
>+              bmsvc.removeItem(itemId);
>+              break;
>+            }
>+          }
>+          //XXX should we remove no more used Smart Bookmarks?

hrm, i don't really like this. what if a user customizes one of these queries? i think it might be a safer path to just add the new ones, since we shouldn't be doing this very often.

it's easy for a user to delete the duplicate item, but impossible for them to recover the one we deleted. (well, outside of doing a full bookmarks restore)

>+  // TEST 2: create smart bookmarks
>+  pref.setIntPref("browser.places.smartBookmarksVersion", 0);
>+  gluesvc.ensurePlacesDefaultQueriesInitialized();
>+  smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
>+  do_check_neq(smartBookmarkItemIds.length, 0);

please explicitly check the amount
Attachment #313591 - Flags: review?(dietrich) → review-
(In reply to comment #16)
> (From update of attachment 313591 [details] [diff] [review])

> hrm, i don't really like this. what if a user customizes one of these queries?
> i think it might be a safer path to just add the new ones, since we shouldn't
> be doing this very often.

do need to remove the annotation from the old queries.
(Assignee)

Comment 18

9 years ago
problem is when we have to change a query because the old version does not work anymore, for example when we changed TAG query to use RESULTS_AS_TAG_QUERY. But on the other side if the user has personalized the query (changing for example the max number of returned records) we don't know and could be lost. 
I could add a new property "forceUpdate: false" to a query to indicate if it should be replaced, this way a change that does not make the old query obsolete will simply add new query and remove anno from the old, one that makes old obsolete will replace it (since old would not work anymore).
(Assignee)

Comment 19

9 years ago
(In reply to comment #16)
> >+  // TEST 2: create smart bookmarks
> >+  pref.setIntPref("browser.places.smartBookmarksVersion", 0);
> >+  gluesvc.ensurePlacesDefaultQueriesInitialized();
> >+  smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
> >+  do_check_neq(smartBookmarkItemIds.length, 0);
> 
> please explicitly check the amount

so, when modifying smart bookmarks, we will have to update the test... i didn't do that for this reason
(In reply to comment #18)
> problem is when we have to change a query because the old version does not work
> anymore, for example when we changed TAG query to use RESULTS_AS_TAG_QUERY. But
> on the other side if the user has personalized the query (changing for example
> the max number of returned records) we don't know and could be lost. 
> I could add a new property "forceUpdate: false" to a query to indicate if it
> should be replaced, this way a change that does not make the old query obsolete
> will simply add new query and remove anno from the old, one that makes old
> obsolete will replace it (since old would not work anymore).
> 

ugh, right. in those cases the user will end up with duplicates *and* brokenness, so lets stick with the current patch for now.

(In reply to comment #19)
> > please explicitly check the amount
> 
> so, when modifying smart bookmarks, we will have to update the test... i didn't
> do that for this reason
> 

ok, that's fine.
Attachment #313591 - Flags: review- → review+
(In reply to comment #18)
> I could add a new property "forceUpdate: false" to a query to indicate if it
> should be replaced, this way a change that does not make the old query obsolete
> will simply add new query and remove anno from the old, one that makes old
> obsolete will replace it (since old would not work anymore).
> 

i don't think adding this kind of hacky complexity is worth it given the edge-case nature of the issue.
(Assignee)

Comment 22

9 years ago
instead we could do something similar to what has been done to avoid hardcoding roots... use a SMART_BOOKMARK_MAX_RESULTS const and translate it in the backend, providing an option to the user. there is another bug about that IIRC.
Comment on attachment 313092 [details]
screenshot - looking for position

Hm, a couple of things are wrong here.

First, Organize bookmarks somehow slipped beneath that first separator, it should be above it.

Second, I think we want a separator after these queries, as they're special.

So what I think we should do is:

 - move Organize Bookmarks above the first separator
 - insert the new queries at index=0
 - add a separator

The result, on a default profile, should look like:

Bookmark This Page
Subscribe to This Page
Bookmark All Tabs
Organize Bookmarks
-----------------------
Bookmarks Toolbar   >
-----------------------
Recently Bookmarked >
Recent Tags         >
-----------------------
Get Bookmark Add-ons
-----------------------
Mozilla Firefox     >
Attachment #313092 - Flags: ui-review?(beltzner) → ui-review-
Bug 427558 has just been filed to add a separator between Organize Bookmarks and Bookmark Toolbar. This would make the top of the menu look like:

Bookmark This Page
Subscribe to This Page
Bookmark All Tabs
-----------------------
Organize Bookmarks
-----------------------
Bookmarks Toolbar   >
-----------------------

This is an extension of bug 408938 and is mentioned in comment 9 of that bug.
(Assignee)

Updated

9 years ago
Blocks: 427558
(Assignee)

Comment 25

9 years ago
thank you Luke, will address here
(Assignee)

Comment 26

9 years ago
Created attachment 314134 [details]
screenshot - new position
Attachment #313092 - Attachment is obsolete: true
Attachment #314134 - Flags: ui-review?(beltzner)
Attachment #314134 - Flags: ui-review?(beltzner) → ui-review+
ermm, since you're about adding even more 'clutter' to the bookmars menu (in addition to the bookmarks toolbar folder), i strongly hope that the 'recently bookmarked' & 'recent tags' entries are

a) deletable in the menu
*and*
b) still being kept in the 'most visited' folder in the bookmarks toolbar if deleted in the menu

sorry about the negative tone, but i don't like this progression... .
(Assignee)

Comment 28

9 years ago
(In reply to comment #27)
> ermm, since you're about adding even more 'clutter' to the bookmars menu (in
> addition to the bookmarks toolbar folder), i strongly hope that the 'recently
> bookmarked' & 'recent tags' entries are
> 
> a) deletable in the menu

Yes, deletable and movable

> b) still being kept in the 'most visited' folder in the bookmarks toolbar if
> deleted in the menu

You can move all queries to a single folder (like current Smart bookmarks) if you want. Most Visited is a query like the others, so you'll have to create a new folder and move all of them inside.

See earlier comments in this bug for the rationale for moving it. That rationale still stays. This is for the default view, users can always customize.
ok, then you have my blessing for checkin ;)
(Assignee)

Comment 31

9 years ago
Created attachment 314153 [details] [diff] [review]
patch v2.2

Fixed menu position, removed annos on no more used Smart Bookmarks, create separator in bookmarks menu if we are creating Smart Bookmarks from ground up.
Asking a new review due to changes.
Attachment #313591 - Attachment is obsolete: true
Attachment #314153 - Flags: review?(dietrich)
Comment on attachment 314153 [details] [diff] [review]
patch v2.2


>+            }
>+            // We don't remove no more used Smart Bookmarks because user

"We don't remove the old Smart Bookmarks" is clearer I think.

r=me
Attachment #314153 - Flags: review?(dietrich) → review+
(Assignee)

Comment 33

9 years ago
Created attachment 314197 [details] [diff] [review]
for check-in
Attachment #314153 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [has patch][needs UI-R beltzner] → [has patch]
(Assignee)

Updated

9 years ago
Whiteboard: [has patch] → [has patch][has review][has ui-review]
(Assignee)

Updated

9 years ago
Flags: in-testsuite?
(Assignee)

Comment 34

9 years ago
NOTE for beta tester users. Old Smart Bookmarks folder should be manually removed, or you can move new queries inside it, but please delete old queries and retain newly created ones.
Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.86; previous revision: 1.85
done
Checking in browser/components/nsIBrowserGlue.idl;
/cvsroot/mozilla/browser/components/nsIBrowserGlue.idl,v  <--  nsIBrowserGlue.idl
new revision: 1.5; previous revision: 1.4
done
Checking in browser/components/places/tests/unit/test_384370.js;
/cvsroot/mozilla/browser/components/places/tests/unit/test_384370.js,v  <--  test_384370.js
new revision: 1.18; previous revision: 1.17
done
Checking in browser/components/places/tests/unit/test_bookmarks_html.js;
/cvsroot/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js,v  <--  test_bookmarks_html.js
new revision: 1.37; previous revision: 1.36
done
Checking in browser/base/content/browser-menubar.inc;
/cvsroot/mozilla/browser/base/content/browser-menubar.inc,v  <--  browser-menubar.inc
new revision: 1.158; previous revision: 1.157
done
Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.289; previous revision: 1.288
done
RCS file: /cvsroot/mozilla/browser/components/places/tests/unit/test_421483.js,v
done
Checking in browser/components/places/tests/unit/test_421483.js;
/cvsroot/mozilla/browser/components/places/tests/unit/test_421483.js,v  <--  test_421483.js
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has review][has ui-review]
(Assignee)

Comment 36

9 years ago
Created attachment 314451 [details] [diff] [review]
additional fix

smartBookmarksVersion should be 0 if the pref is not found!
Attachment #314451 - Flags: review?(dietrich)
(Assignee)

Updated

9 years ago
Flags: in-testsuite? → in-testsuite+
Comment on attachment 314451 [details] [diff] [review]
additional fix

r=me for followup
Attachment #314451 - Flags: review?(dietrich) → review+

Comment 38

9 years ago
Created attachment 314465 [details]
screentshot

"Most Visited" folder icon should be the same folder icon as "Recently ..." ?

old "smart bookmarks"
http://img241.imageshack.us/img241/7153/sb1jr9.jpg
followup checked in:

Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.88; previous revision: 1.87
done
(Assignee)

Comment 40

9 years ago
(In reply to comment #38)
> "Most Visited" folder icon should be the same folder icon as "Recently ..." ?

that feels like a problem in the toolbar view that is not appying special styling to queries... should be filled as a new bug imo

Comment 41

9 years ago
beltzner, this looks like moving a string from one place to another, in particular from one CSS constrained size to another CSS constrained size.

That is, different crop lengths apply to the existing string.

marcoos notes that this makes things easier for most locales, but I don't fully understand when which string moved from where to where.

If my analysis is roughly right, could we get an announcement in .l10n explaining what to look for and how to reproduce?

This is really subtle, but I know that people go for worse translations of items to not get them cropped with ellipses due to theme CSS, so I think there is some l10n impact involved.

Comment 42

9 years ago
Clint, this should show up in Minotaur reports, too.

Comment 43

9 years ago
Marek stepped up and posted http://groups.google.de/group/mozilla.dev.l10n/browse_thread/thread/2f405d181d7cd841.

Comment 44

9 years ago
(In reply to comment #42)
> Clint, this should show up in Minotaur reports, too.
> 
I believe they will be visible there.  I've been following this bug, and will make certain of it when I dive into Minotaur to make it export the default protocol handler information later this week.

Comment 45

9 years ago
Marco or Dietrich, can you please clarify the behavior of this patch in the following scenario using Foxmarks bookmarks synchronization:
(I just went through it, but it will of course happen each time a user installs a new Firefox version that revs the smartBookmarksVersion)

1) Firefox on computer 1 runs smart bookmarks update, creates new queries, massages annotations, sets version pref to new value.

2) On computer 2 Foxmarks runs synchronization, before Firefox can be updated to the new build/release. Foxmarks syncs new queries to places.sqlite, but smartBookmarksVersion pref is of course not incremented.

3) New build/release runs for the first time, runs smart query migration, because version pref is not up-to-date, although the queries are up-to-date.

4) the new/current smart bookmarks are apparently duplicated.

Questions:
A) is it possible to access the stored annotations for a bookmark, to see what's going on here (SQLite Explorer)?
B) Assuming Foxmarks syncs annotations at all, are "foxmarks'" smart bookmarks "invalidated" by removing the annotations, and the new queries created again?
(Assignee)

Comment 46

9 years ago
as you can see in the code we do:
var smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
with
const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";

So when syncing you should bring with you that annotation, or we will not find any smart bookmark, and they will be rebuilt by zero in their default position.

Comment 47

9 years ago
So to me it looks like only the parent, position and itemID are preserved, the title, uri properties are carried over from the smartBookmarks array.

I can only report that after I ran through the smart bookmark update on two computers in roughly the sequence outlined in comment 45 I had all smart bookmarks duplicated: Most Visited on the toolbar, and Recent Tags, Recently Bookmarked in the menu.

Foxmarks for Firefox 3 is here: http://beta.foxmarks.com, in case QA or someone wants to try... it is a Recommended Extension on AMO.

Comment 48

9 years ago
Before I get on everybody's nerves:
using a third machine I confirmed that the Places/SmartBookmarks annotation doesn't make it to the other machines. I contacted Foxmarks about the issue.
(Assignee)

Comment 49

9 years ago
(In reply to comment #48)
> I contacted Foxmarks about the issue.

thank you very much!

Comment 50

9 years ago
(In reply to comment #42)
> Clint, this should show up in Minotaur reports, too.
> 
Axel, I've been testing this change with Minotaur, all the smart bookmark titles show up properly, though I can't tell if they have been clipped to ellipsis or not by the theme (as I get the values from an interface rather than screen scraping).

Minotaur also now reports the browser.places.smartBookmarksVersion preference, since I wasn't sure if you wanted that or not and it was easy to include.  

Let's take the Minotaur specific discussion to bug 396132 which is where the changes for this will land in the minotaur tool. (I'm rolling them in together with the protocol handler changes).

Comment 51

9 years ago
(In reply to comment #49)
> (In reply to comment #48)
> > I contacted Foxmarks about the issue.
> 
> thank you very much!
> 

My conversations with Todd Agulnick from Foxmarks tended towards changing nothing on Foxmarks side, the effort vs. benefit ratio of syncing the "hidden bookmark title" in the annotation seems not high enough for them.

However, we noticed that e.g. the "Most Visited" query is synced not as
"place:queryType=0&sort=8&maxResults=10" but as
"place:sort=8&maxResults=10"
They use nsINavHistoryResultNode.uri to read and nsINavBookmarksService.changeBookmarkURI to update bookmarks. Is that not the right way to do it?
(Assignee)

Comment 52

9 years ago
(In reply to comment #51)
> My conversations with Todd Agulnick from Foxmarks tended towards changing
> nothing on Foxmarks side, the effort vs. benefit ratio of syncing the "hidden
> bookmark title" in the annotation seems not high enough for them.

That is not an hidden bookmark title, it's a string identifier to recognize a query from another.

> However, we noticed that e.g. the "Most Visited" query is synced not as
> "place:queryType=0&sort=8&maxResults=10" but as
> "place:sort=8&maxResults=10"
> They use nsINavHistoryResultNode.uri to read and
> nsINavBookmarksService.changeBookmarkURI to update bookmarks. Is that not the
> right way to do it?

IIRC querytype=0 should be the default, so the uri should work

Comment 53

9 years ago
(In reply to comment #52)
> > However, we noticed that e.g. the "Most Visited" query is synced not as
> > "place:queryType=0&sort=8&maxResults=10" but as
> > "place:sort=8&maxResults=10"
> > They use nsINavHistoryResultNode.uri to read and
> > nsINavBookmarksService.changeBookmarkURI to update bookmarks. Is that not the
> > right way to do it?
> 
> IIRC querytype=0 should be the default, so the uri should work
> 

Yeah, of course it does... but the user looks at the bookmarks on both his machines and expects to see the same properties. Why does the Places API and the Places Library expose different values for the URI? What property (of which interface) is fed into the Library tree?
I've verified that the pre-populated bookmarks have been moved as expected. This has been the case for a while but I checked specifically with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre.
Status: RESOLVED → VERIFIED
Probably this caused bug 432323 (Trying to delete the "Smart Bookmarks" folder crashes Firefox [@ nsNavHistoryResult::OnItemChanged]).
Blocks: 413693
(Assignee)

Comment 56

9 years ago
(In reply to comment #55)
> Probably this caused bug 432323 (Trying to delete the "Smart Bookmarks" folder
> crashes Firefox [@ nsNavHistoryResult::OnItemChanged]).

i doubt, there's no code involving the old smart bookmarks folder, it simpy didn't get touched (indeed it had to be removed manually) and we create new queries. The crash could instead involve some relation between the toolbar view and a folder containing queries.

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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.