Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Save resilience #2961

Merged
merged 3 commits into from
Jun 5, 2017
Merged

Save resilience #2961

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
308913b
Add more structured error messages to bad image URLs
ianb Jun 2, 2017
3c2323d
Make re-saving an image work
ianb Jun 2, 2017
dbeb56d
Fix #2957, make empty selections report an error
ianb Jun 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion addon/webextension/background/senderror.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ this.senderror = (function() {
MY_SHOTS: {
title: browser.i18n.getMessage("selfScreenshotErrorTitle")
},
EMPTY_SELECTION: {
title: browser.i18n.getMessage("emptySelectionErrorTitle")
},
generic: {
title: browser.i18n.getMessage("genericErrorTitle"),
info: browser.i18n.getMessage("genericErrorDetails"),
Expand Down Expand Up @@ -112,7 +115,9 @@ this.senderror = (function() {
if (!errorObj.noPopup) {
exports.showError(errorObj);
}
exports.reportError(errorObj);
if (!errorObj.noReport) {
exports.reportError(errorObj);
}
});

return exports;
Expand Down
8 changes: 8 additions & 0 deletions addon/webextension/selector/shooter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ this.shooter = (function() { // eslint-disable-line no-unused-vars
// isSaving indicates we're aleady in the middle of saving
// we use a timeout so in the case of a failure the button will
// still start working again
if (Math.floor(selectedPos.left) == Math.floor(selectedPos.right) ||
Math.floor(selectedPos.top) == Math.floor(selectedPos.bottom)) {
let exc = new Error("Empty selection");
exc.popupMessage = "EMPTY_SELECTION";
exc.noReport = true;
catcher.unhandled(exc);
return;
}
const uicontrol = global.uicontrol;
let deactivateAfterFinish = true;
if (isSaving) {
Expand Down
2 changes: 2 additions & 0 deletions docs/error-handling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,5 @@ Note that any information will be serialized as JSON. Specifically `undefined`
If you add `exc.popupMessage = "Something happened"` then that detail will be added. Be careful about localization here.

Add `exc.noPopup = true` if you don't want the user notified about the error (but the error will still be sent to Sentry).

Add `exc.noReport = true` if you don't want the error reported to Sentry.
2 changes: 2 additions & 0 deletions locales/en-US/webextension.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ loginErrorDetails = We couldn’t save your shot because there is a problem with
unshootablePageErrorTitle = We can’t screenshot this page.
unshootablePageErrorDetails = This isn’t a standard Web page, so you can’t take a screenshot of it.
selfScreenshotErrorTitle = You can’t take a shot of a Firefox Screenshots page!
# Fired when someone makes a zero-width or zero-height selection
emptySelectionErrorTitle = Your selection is too small
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flodolo We're aiming for 55, but have a new string to add. Is it possible to get strings uplifted after 55 graduates to Beta?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localization happens in GitHub, outside of mozilla-central, so you're not bound to trains: the content that you land in mozilla-central is not exposed to localizers, so no problem with uplifts.

Having said that, the sooner you land, the better. And if you're planning to enable in 55, website's localization is getting kind of late.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very obscure error: the user has the resize the selection down to zero height or width to encounter it, so I think if it doesn't get localized the impact will be very small.

genericErrorTitle = Whoa! Firefox Screenshots went haywire.
genericErrorDetails = We’re not sure what just happened. Care to try again or take a shot of a different page?
# Section for onboarding strings
Expand Down