405936 - export "unfiled bookmarks" folder to html

Status: VERIFIED FIXED

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

Description

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

export "unfiled bookmarks" folder to html

Comment 1

[Mike Connor [:mconnor]]

hmm, so, where does this go in an HTML export?  we should back this up, but I don't think its critical that we do this as long as the JSON backup works...

Comment 2

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

I'm not sure I agree.  first, I believe we will continue to export to html, even after we have json backup.  (is that correct dietrich?)

the html format is useful for interchange between browsers vendors and even between versions of firefox (and other things, I'm sure).

as for where it should live, I think it would be acceptable if we dumped it as top level folder.

a separate bug could be to write out something unique in the html for the untitle folder (that wouldn't hurt fx 2 and other browser on import) that fx 3 could use to indicate that the contents of this folder should go in the special unfiled root, and then fix our import / and backup restore code to handle it.

Comment 3

[Mike Connor [:mconnor]]

Ok, I'm still not sure I agree, but I don't disagree strong enough to make a good case for not supporting this.  Nowhere near the top priority though.

Comment 4

[mmortal03]

Does this bug imply that unfiled bookmarks are not currently being backed up by Minefield?  I ran into that problem yesterday, and luckily had a previous set backed up that I hadn't moved to unfiled yet.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

The "Unfiled Bookmarks" folder is not in the nightly backups right now.

Comment 6

[Mike Connor [:mconnor]]

Not blocking on this bug for final ship. Would take a safe enough patch if one comes through.

Comment 7

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

Hm. This feels like a pretty important scenario to cover off, since we're going to be telling users that bookmarking a page is now as easy as clicking the star. If they export their bookmarks, whether or not they took the time to file them should be irrelevant - they should still have them available.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I think we will lose a lot of user bookmark data this way, typically from the people who are least likely to have other ways of retrieving it (having other backups and the knowledge to restore them, etc.).  Relying on the distinction between "export" and "backup" to save these people seems like it's going to leave a pretty bad taste in the mouths of people who we're marketing FF3's bookmarking improvements to.  (Including, explicitly, the improved reliability!)

Comment 9

[Dietrich Ayala (:dietrich)]

Sure, this is not so hard to fix, and would ease things for folks relying on the HTML export/import.

However, the HTML format will still be lossy, as it doesn't have tags, nor annotations.

(In reply to comment #8)
> people who are least likely to have other ways of retrieving it (having other
> backups and the knowledge to restore them, etc.). 

Why are these users exporting as HTML from Firefox 3?

Bug 422919 will make clear that Export == HTML. However, this still doesn't communicate that HTML != all of Places' data.

We should design the menu such that it's clear that HTML is the secondary-suboptimal choice, used only for backwards compatibility and simple publishing.

Perhaps rearrange the menu something like:

Import and Backup
 Save
 Restore
  XXX dates
  Choose file
 ------------
 HTML
  Import
  Export

* this removes the import/export vs backup/restore distinction from the top level of the menu
* it emphasizes JSON by putting it first and describing it more generally as "Save"
* it makes clear that HTML is separate from our save/restore mechanism by calling it out by name

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Save as...

 [v Web Page, simple]

produces warning label when chosen:

|
|  /\
| /!!\   Oh dear!  This is not a complete backup!
| ----


?

Comment 11

[mmortal03]

(In reply to comment #9)
> Sure, this is not so hard to fix, and would ease things for folks relying on
> the HTML export/import.
> 
> However, the HTML format will still be lossy, as it doesn't have tags, nor
> annotations.
> 

Yeah, but losing their links is what most people we are talking about are going to care about.  The lossless idea with the json format could come into play here after the fact, but that is still a separate bug at the moment.

Comment 12

[Dietrich Ayala (:dietrich)]

(In reply to comment #10)
> Save as...
> 
>  [v Web Page, simple]
> 
> produces warning label when chosen:
> 
> |
> |  /\
> | /!!\   Oh dear!  This is not a complete backup!
> | ----
> 
> 
> ?
> 

are you proposing this as an alternative to comment #9? i think it's ok in addition to a menu rearrange, but in itself is not a full solution.

the menu choices should be designed such that the user does not have to resort to trial and error to see which option they actually want.

Comment 13

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

Discussion of comment 9 through comment 12 should really be in bug 422919; let's not morph this into both the UI solution and the underlying solution.

This bug is about whether or not the (admittedly lossy) HTML export format should include items in the Unsorted Bookmarks folder. The more I think about this, the more I think it's important for us to make it easy for users to get their bookmarks out of the product in a way that's useful in all other products (which the HTML format is) and that not including unfiled bookmarks would be seen as dataloss.

Marking P2 blocking.

Comment 14

[Asaf Romano (gone)]

Attachment: patch

Comment 15

[Asaf Romano (gone)]

Attachment: include a test

Comment 16

[Asaf Romano (gone)]

Attachment: but without other patches

Comment 17

[Dietrich Ayala (:dietrich)]

Comment on attachment 312515 [details] [diff] [review]
but without other patches


>@@ -2268,16 +2289,22 @@ nsPlacesImportExportService::RunBatched(
> 
>     PRInt64 toolbarFolder;
>     rv = mBookmarksService->GetToolbarFolder(&toolbarFolder);
>     NS_ENSURE_SUCCESS(rv,rv);
> 
>     rv = mBookmarksService->RemoveFolderChildren(toolbarFolder);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>+    PRInt64 unfiledBookmarksFolder;
>+    rv = mBookmarksService->GetUnfiledBookmarksFolder(&unfiledBookmarksFolder);
>+    NS_ENSURE_SUCCESS(rv,rv);
>+
>+    rv = mBookmarksService->RemoveFolderChildren(unfiledBookmarksFolder);
>

check rv plz


> -->
>     </DL>
>+    <DT><H3 UNFILED_BOOKMARKS_FOLDER="true">Unsorte Bookmarks</H3>

nit: "Unsorted"

r=me

Comment 18

[Asaf Romano (gone)]

mozilla/browser/components/places/src/nsPlacesImportExportService.cpp 1.62
mozilla/browser/components/places/tests/unit/bookmarks.preplaces.html 1.7
mozilla/browser/components/places/tests/unit/test_bookmarks_html.js 1.33

Comment 19

[pal-moz]

Attachment: screenshot

20080331_1944_firefox-3.0pre.en-US.win32.zip

with old profile, "Unfiled Bookmarks" is created.
with new profile, this is "Unsorted Bookmarks".

intended ?

Comment 20

[Reed Loden [:reed]]

This caused TUnit failures. Backed out.

Comment 21

[Asaf Romano (gone)]

Thanks reed.

(In reply to comment #19)
> Created an attachment (id=312847) [details]
> screenshot
> 
> 20080331_1944_firefox-3.0pre.en-US.win32.zip
> 
> with old profile, "Unfiled Bookmarks" is created.
> with new profile, this is "Unsorted Bookmarks".
> 
> intended ?
> 
Yes, it's the name of the folder within the organizer, we didn't right a unsorted->unfiled migrator ;)

Comment 22

[Asaf Romano (gone)]

Attachment: fix the json test

Comment 23

[Asaf Romano (gone)]

landed again:
mozilla/browser/components/places/src/nsPlacesImportExportService.cpp 1.64
mozilla/browser/components/places/tests/unit/bookmarks.preplaces.html 1.9
mozilla/browser/components/places/tests/unit/test_384370.js 1.15
mozilla/browser/components/places/tests/unit/test_bookmarks_html.js 1.35

Comment 24

[Tracy Walker [:tracy]]

Verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042407
Minefield/3.0pre

Comment 25

[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