381338 - Allow import of bookmarks from other profiles that we support migration from.

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Mark Banner (:standard8)]

I'd already implemented this in some early patches on bug 329744. Now that's landed we can prepare this to land at switchover time for suiterunner.

Comment 1

[Mark Banner (:standard8)]

Attachment: Possible patch

This works, but when selecting from file, you can see the migrator wizard advancing to the next page before the file open dialog comes up.

I'm not sure why this happens yet, it seems like these lines aren't doing their job:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/profile/migration/migration.js&rev=1.1&mark=154-155#147

Comment 2

[Mark Banner (:standard8)]

Attachment: Patch v2

Allow import of bookmarks from existing profiles/apps (where the profile migrator supports them).

The option is available under tools->import of the bookmarks manager.

Note that I've added some bits to stop the wizard advancing to the next page when the from file option is chosen in case the file dialog is slow to come up and the wizard is slow to cancel (as seen on my system), this also prevents an error occurring due to the next page trying to use something that isn't defined because we've selected to import from a file.

Requesting approvals now so we're ready to land this at or soon after the suiterunner switch on trunk.

Comment 3

[[e-mail]]

Comment on attachment 265439 [details] [diff] [review]
Patch v2

>+#ifdef XP_MACOSX
>+    var features = "centerscreen,chrome,resizable=no";
This isn't going to work. The wizard has to be modal, otherwise we won't know whether the "File" import source has been chosen.

>+      // Don't let the wizard migrate to the next page event though we've
>+      // called cancel - cancel may not get processed first.
Odd, because it tries very hard to close the window!

Comment 4

[[e-mail]]

Comment on attachment 265439 [details] [diff] [review]
Patch v2

So, for some reason, it's offering the choice of importing from Thunderbird, despite my never having used it, but not IE :-\

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> (From update of attachment 265439 [details] [diff] [review])
> So, for some reason, it's offering the choice of importing from Thunderbird,
> despite my never having used it, but not IE :-\

It won't offer IE because we haven't written the profile migrator for it yet.

Note sure why its offering Thunderbird - I had Thunderbird profiles available for migration on linux, but it didn't offer them in the bookmarks manager. So either its a windows bug, or its some incorrect logic.

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #3)
> (From update of attachment 265439 [details] [diff] [review])
> >+      // Don't let the wizard migrate to the next page event though we've
> >+      // called cancel - cancel may not get processed first.
> Odd, because it tries very hard to close the window!

Yep, but for some reason my linux build definitely showed it moving on first and generating an error before getting round to dealing with the cancel.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 265439 [details] [diff] [review]
Patch v2

New patch coming up later that removes the ifdef.

Comment 8

[Mark Banner (:standard8)]

Attachment: Patch v3

Neil said on irc that the thunderbird problem went away after a rebuild.

This patch removes the ifdef that was previously there as it won't do any good as per Neil's comment 3.

Comment 9

[[e-mail]]

Comment on attachment 265545 [details] [diff] [review]
Patch v3

>+    window.openDialog("chrome://communicator/content/migration/migration.xul",
>+                      "migration", "modal,centerscreen,chrome,resizable=no",
>+                      "bookmarks");
Sorry, I should have mentioned before that I don't like window names, that is to say, the second parameter to openDialog should always be the empty string.

Comment 10

[Mark Banner (:standard8)]

Patch checked in (with changed window title) -> fixed.