Closed Bug 861546 Opened 12 years ago Closed 12 years ago

Preferences.jsm does not need to guard against clearUserPref throwing

Categories

(Toolkit :: General, defect)

Product:
Toolkit
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla24

People

(Reporter: Gavin, Assigned: riadh.chtara)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 4 obsolete files)

patch with header information
1.59 KB, patch
jaws
: review+
Details | Diff | Splinter Review
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.
Whiteboard: [good first bug]
hey, I will fix it.
Attached patch delete non needed guard (obsolete) — — Splinter Review
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.
Attached patch deleting _reset (obsolete) — — Splinter Review
Attachment #751721 - Flags: review+
Attachment #751721 - Flags: feedback+
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 #751721 - Flags: review+
Attachment #751721 - Flags: feedback+
Attachment #751206 - Attachment is obsolete: true
Sorry for that
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
Attached patch patch with header information (obsolete) — — Splinter Review
Attachment #751781 - Flags: review?
Attachment #751781 - Flags: review? → review?(dietrich)
@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 <riadh.chtara@gmail.com> >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
Attached patch patch with header information (obsolete) — — Splinter Review
Attachment #751781 - Attachment is obsolete: true
wait a second
Attachment #751788 - Attachment is obsolete: true
That s awesome
https://hg.mozilla.org/mozilla-central/rev/a2bcdf9b6a2a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: