I can't reproduce this on Win XP, latest trunk hourly. See http://img264.imageshack.us/img264/6664/importpc1.png
Hmm, yeah, with a new profile, it seems to work. However, it didn't import anything the first three tries. After that, it worked. Not sure what's going on.
Ok, it seems that I'm not able to import the bookmarks file at all with a new profile. But when I've deleted the already existing bookmarks, then I'm able to import the bookmarks file. I have no idea what's going on.
I see them also with a new profile but with my old set of bookmarks.
import bug + bookmarklets = blocking?
Well, I only hope you can reproduce any of the problems I got.
Assignee: nobody → sspitzer
Target Milestone: --- → Firefox 3 alpha6
> dietrich / mano / mconnor: comments? dietrich agreed with me over irc that we should stop importing / exporting item_id, and revert to the fx 2 behaviour of import always appending bookmarks. that will fix this bug.
Status: NEW → ASSIGNED
morphing the summary. on track to fixing this for A6. I've got the easy patch to stop exporting item_ids, working on the rest of it. note I also have to worry about not merging bookmarks into the personal toolbar folder.
11 years ago
Created attachment 269550 [details] [diff] [review] fix v1 don't merge bookmarks or folders, only append. in my tests, this makes import behave just like fx2. i think we need to do this for now, due to the data-loss issues of merging. we can tackle merging as a feature later if users ask for it.
Created attachment 269678 [details] [diff] [review] patch v1 oops, duplication of effort over the weekend. I'll go review dietrich patch.
in fx 2, if you manually import, the manually imported personal toolbar folder gets appended as a "regular" folder. with your patch, won't the items of the imported personal toolbar folder get merged into the existing personal toolbar folder? I may try to convince dietrich to review my patch instead, as I think I've fixed that issue, and fixed the problem with our fake icon urls (see bug #380449) and removed the code that exports item_ids, (bug #381225, or that may just be a dup of this bug.)
also note the BOOKMARKSS_MENU part of this fix. It appears that when this file was moved from mozilla/toolkit/components/places/src/nsBookmarksHTML.cpp to mozilla/browser/components/places/src/nsPlacesImportExportService.cpp, that extra "s" slipped in. (compare 1.35 of nsBookmarksHTML.cpp to 1.1 of nsPlacesImportExportService.cpp and see the patch in bug #376008) The place it really matters is here: -static const char kBookmarksRootAttribute = " BOOKMARKSS_MENU=\"true\""; +static const char kBookmarksRootAttribute = " BOOKMARKS_MENU=\"true\""; So that should mean that when the trunk was exporting to bookmarks.html (for firefox 2), firefox 2 may have had problems, since there was no BOOKMARKS_MENU attribute. Additionally, when the trunk was importing it's own bookmarks.html (on schema change), we may have had similar issues, as the import code in nsPlacesImportExportService.cpp was expecting one "s": #define KEY_BOOKMARKSSMENU_LOWER "bookmarks_menu" (The rest of the extra "s" changes are internal to the code)
Created attachment 269700 [details] [diff] [review] updated patch, fix one more PRInt64 cast issue (date)
Comment on attachment 269700 [details] [diff] [review] updated patch, fix one more PRInt64 cast issue (date) patch needs a little work / cleanup before I seek review.
Attachment #269700 - Attachment is obsolete: true
> firefox 2 may have had problems I take that back, since fx 2 had no idea about the BOOKMARKS_MENU attribute > Additionally, when the trunk was importing it's own bookmarks.html > (on schema change), we may have had similar issues I take that back, too. I'm not seeing us export BOOKMARKS_MENU attribute (or the PLACES_ROOT attribute). I'll continue testing this patch, and then spin off bugs about the BOOKMARKS_MENU and PLACES_ROOT
11 years ago
Attachment #269711 - Flags: review?(dietrich)
> spin off bugs about the BOOKMARKS_MENU and PLACES_ROOT see bug #385835
Comment on attachment 269711 [details] [diff] [review] updated patch thanks, looks good, r=me.
Attachment #269711 - Flags: review?(dietrich) → review+
fixed. Checking in nsPlacesImportExportService.cpp; /cvsroot/mozilla/browser/components/places/src/nsPlacesImportExportService.cpp,v <-- nsPlacesImportExportService.cpp new revision: 1.23; previous revision: 1.22 done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
again, a test case could be written here to test these changes. for nsIPlacesImportExportService, we could check that: a) with "importHTMLFromFile(x, false);", importing appends b) with "importHTMLFromFile(x, true);", importing replaces c) exportHTMLToFile() doesn't write out HTML with ITEM_IDs. In fact, there are a lot of import / export tests that we should be doing. I'll log a spin of bug on that.
> I'll log a spin of bug on that. See bug #385859
Flags: blocking-firefox3? → blocking-firefox3+
Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0 works OK in RC1
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.