Closed Bug 861546 Opened 7 years ago Closed 6 years ago
.jsm does not need to guard against clear User Pref throwing
I noticed one minor issue with the code being moved into Preferences.jsm in toolkit in bug 848519. I believe it was written before bug 487059, so its _reset method specifically swallows NS_ERROR_UNEXPECTED exceptions. This isn't necessary anymore since clearUserPref now never throws.
hey, I will fix it.
delete non needed guard
Assignee: nobody → riadh.chtara
Status: NEW → ASSIGNED
Comment on attachment 751206 [details] [diff] [review] delete non needed guard Review of attachment 751206 [details] [diff] [review]: ----------------------------------------------------------------- If _reset is only this one line, it's probably better to remove this function and change it's callers to just call clearUserPref.
Thanks for the review I updated
Comment on attachment 751721 [details] [diff] [review] deleting _reset I didn't spend enough time on my previous drive-by feedback to know if this is good or not. Please don't set a review+ or feedback+ unless someone has already granted you the approval.
Attachment #751206 - Attachment is obsolete: true
Sorry for that
Comment on attachment 751721 [details] [diff] [review] deleting _reset Thanks! This is already covered by a test: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/tests/xpcshell/test_Preferences.js#178
Riadh, can you please update your patch to have the necessary header information so someone else can check it in for you? You can follow the steps here, https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
@Gavin Sharp: you are welcome @jaws: I just done that
Attachment #751781 - Flags: review?(dietrich) → review+
Comment on attachment 751781 [details] [diff] [review] patch with header information ># HG changeset patch ># Parent 0c1663454e490c4215e9df3cd6eef1f8025893d8 ># User Riadh Chtara <firstname.lastname@example.org> >Bug 861546 - Preferences.jsm is changed so it does not need to guard against clearUserPref throwing; r=reviewers For the bug summary, this should be r=gavin instead of r=reviewers.
Attachment #751781 - Flags: review+
Attachment #751721 - Attachment is obsolete: true
wait a second
That s awesome
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.