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

RESOLVED FIXED in Camino1.5

Status

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: alqahira, Assigned: froodian)

Tracking

({fixed1.8.1.2})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.2
Bug Flags:
camino1.1b1 +

Details

(URL)

Attachments

(2 attachments)

------- 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 ;)
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 ;)
Flags: camino1.1b1?
(Assignee)

Comment 2

12 years ago
Created attachment 249802 [details] [diff] [review]
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?
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #249802 - Flags: review?(stuart.morgan)
(Assignee)

Updated

12 years ago
Whiteboard: [New Strings Needed]
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?
(Assignee)

Comment 4

12 years ago
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

12 years ago
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.
Attachment #249802 - Flags: review?(stuart.morgan) → review+
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 on attachment 249802 [details] [diff] [review]
Patch (checked in on trunk and 1.8.1.2)

+- (void)restoreBackupBookmarksAltert;

fix typo

sr=pink
Attachment #249802 - Flags: superreview+
"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 10

12 years ago
Checked into the MOZILLA_1_8_BRANCH. Does this need to find its way into trunk as well?
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [New Strings Needed] → fixed.1.8.1
(Assignee)

Comment 11

12 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

12 years ago
Whiteboard: fixed.1.8.1
(Assignee)

Updated

12 years ago
Attachment #249802 - Attachment description: Patch → Patch (checked in on trunk and 1.8.1.2)
(Assignee)

Comment 12

12 years ago
Created attachment 251674 [details] [diff] [review]
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";
Attachment #251674 - Flags: review?(stuart.morgan)
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

12 years ago
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.
Attachment #251674 - Flags: review?(stuart.morgan) → review+
Comment on attachment 251674 [details] [diff] [review]
Patch

rs=pink with smorgan's suggested change.
Attachment #251674 - Flags: superreview+
(Assignee)

Comment 16

12 years ago
Fixed on trunk and MOZILLA_1_8_BRANCH
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Keywords: fixed1.8.1.2
Resolution: --- → FIXED
Filed bug 368096 to make sure we get a m.org url for this.
Flags: camino1.1b1? → camino1.1b1+
You need to log in before you can comment on or make changes to this bug.