364496 - Display a warning during bookmarks recovery alerting the user we're restoring

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

------- Comment #49 From Smokey Ardisson (unreliable; no bugmail)  2006-12-15 20:32 PST  [reply] -------

One thing I noticed when testing the 1.8.0 branch patch for bug 356708 is that
we don't provide any error message when we restore from backup (on any branch);
IOW, if we successfully restore in cases of corruption, the user never knows
there was corruption. 

This is perfect in normal cases, but it means we've now patched around this bug
so successfully that users will never know to look in their Console.log for
info to help us solve the actual problem.  Do we want to show an error on
development branches to prompt user data reporting in the
successful-restore-from-backup case (to be removed before release, of course)?

---
Or do we want to remove it before release?

Stuart said this was just a dialogue in the recovery path (and if someone else could fix it soon, that would be good ;)

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since this requires new UI/strings, it needs to make beta (even if we might eventually remove it); however, to get data collection going again, we really need it sooner ;)

Comment 2

[froodian (Ian Leue)]

Attachment: Patch (checked in on trunk and 1.8.1.2)

This will need two new strings.  I was thinking something like

"RestoreBackupBookmarksAltert" = "Restoring Bookmarks From Backup"
"RestoreBackupBookmarksMsg" = "Your bookmarks file was damaged, and has been replaced with a backup.  Please report any error messages from Camino in your console log to the Camino team."

Should we include an email or URL too?

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not fond of how the second sentence in the second string reads.  I'm also concerned that we'll get mass dumping of Console.logs, and there's no good way to explain what things we need in a concise manner for the alert.

Given that, I wonder if it might not be better to say:

"RestoreBackupBookmarksMsg" = "Your bookmarks file was damaged, and has been
replaced with a backup.  For more information on how to provide details that will help the Camino team solve the problem, please visit http://wiki.caminobrowser.org/Damaged_Bookmarks"

and have that page provide details about finding the Console.log and extracting the useful info from it?

Comment 4

[froodian (Ian Leue)]

I like the idea, but it still reads strangely.  It also occurs to me that since Camino's a web browser... we could just offer to open the page for them. Something like:

"RestoreBackupBookmarksMsg" = "Your bookmarks file was damaged, and has been
replaced with a backup.  Would you like more information on how to provide details that will help the Camino team solve the problem?"

(I don't care if Camino's broken) (More Information)

This would need a new patch to provide the second button, and (obviously) better text for the cancel button.  The trick is making it clear in the text that they'll just be visiting a site, and then they'll be able to choose what to send us - we don't want people thinking we'll just scrape their bookmarks file full of pr0n.

Comment 5

[Stuart Morgan]

Comment on attachment 249802 [details] [diff] [review]
Patch (checked in on trunk and 1.8.1.2)

>+- (void)restoreBackupBookmarksAltert;

Change this to "showRestoredBookmarksAlert" (make sure to fix the 'Altert' typo), and r=me assuming we go with that model.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

It would be really nice to get this for a2, and I think I'd prefer the "send them to a webpage" method, but I'll happily go along with comment 2 to have this make a2.  Some notification/data recovery is better than none.

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 249802 [details] [diff] [review]
Patch (checked in on trunk and 1.8.1.2)

+- (void)restoreBackupBookmarksAltert;

fix typo

sr=pink

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

"RestoreBackupBookmarksMsg" = "Your bookmarks file was damaged, and has been replaced with a backup.  To provide details that will help the Camino team solve the problem, please visit http://wiki.caminobrowser.org/QA:Damaged_Bookmarks"

is fine with me.  Note the "QA" in the URL; let's put this page in a reasonably accurate location not at the top level of the wiki ;)

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

First cut of the wiki text/instructions up at http://wiki.caminobrowser.org/QA:Damaged_Bookmarks

Comment 10

[Nick Kreeger]

Checked into the MOZILLA_1_8_BRANCH. Does this need to find its way into trunk as well?

Comment 11

[froodian (Ian Leue)]

Checked in on trunk - it'll be easier to make changes later this way, and we'll want any users adventurous enough to use trunk to see the alert too.  Reopening to discuss the approach in comment 4.

Comment 12

[froodian (Ian Leue)]

Attachment: Patch

This patch provides a two-button alert, which opens the damaged bookmarks page if the user chooses "More Information", and does nothing if the user chooses "Cancel."

To Localizable.strings:
- "RestoredBookmarksMsg" = "Your bookmarks file was damaged, and has been replaced with a backup.  To provide details that will help the Camino team solve the problem, please visit\n\nhttp://wiki.caminobrowser.org/QA:Damaged_Bookmarks";
+ "RestoredBookmarksMsg" = "Your bookmarks file was damaged, and has been replaced with a backup.  Would you like more information on how to provide details that will help the Camino team solve this problem?";
+ "More Information" = "More Information";

To WebsiteDefaults.strings:
+ CorruptedBookmarksDefault = "http://wiki.caminobrowser.org/QA:Damaged_Bookmarks";

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Should More Information have an elipsis?  It's not technically opening up a new chrome window/sheet, but it is presenting another window so to speak.

Also, how about "Would you link to visit a web page with more information..." (see the last part of comment 4)?

Comment 14

[Stuart Morgan]

Comment on attachment 251674 [details] [diff] [review]
Patch

>+  if (NSRunAlertPanel(NSLocalizedString(@"RestoredBookmarksAlert", nil),
>+                      NSLocalizedString(@"RestoredBookmarksMsg", nil),
>+                      NSLocalizedString(@"More Information", nil),
>+                      NSLocalizedString(@"CancelButtonText", nil),

Change "More Information" to something like "RestoredBookmarksInfoButton", and r=me. I agree that we should probably have an ellipsis on the button text.

Comment 15

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 251674 [details] [diff] [review]
Patch

rs=pink with smorgan's suggested change.

Comment 16

[froodian (Ian Leue)]

Fixed on trunk and MOZILLA_1_8_BRANCH

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Filed bug 368096 to make sure we get a m.org url for this.