Preferences.jsm does not need to guard against clearUserPref throwing

RESOLVED FIXED in mozilla24

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Gavin, Assigned: riadh.chtara)

Tracking

Trunk
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 4 obsolete attachments)

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.

Updated

6 years ago
Whiteboard: [good first bug]
(Assignee)

Comment 1

6 years ago
hey,
I will fix it.
(Assignee)

Comment 2

6 years ago
Created attachment 751206 [details] [diff] [review]
delete non needed guard

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.
(Assignee)

Comment 4

6 years ago
Created attachment 751721 [details] [diff] [review]
deleting _reset
Attachment #751721 - Flags: review+
Attachment #751721 - Flags: feedback+
(Assignee)

Comment 5

6 years ago
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+
(Assignee)

Comment 7

6 years ago
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
(Assignee)

Comment 10

6 years ago
Created attachment 751781 [details] [diff] [review]
patch with  header information
Attachment #751781 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #751781 - Flags: review? → review?(dietrich)
(Assignee)

Comment 11

6 years ago
@Gavin Sharp: you are welcome

@jaws: I just done that
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+
(Assignee)

Comment 13

6 years ago
Created attachment 751788 [details] [diff] [review]
patch with header information
Attachment #751781 - Attachment is obsolete: true
(Assignee)

Comment 14

6 years ago
wait a second
(Assignee)

Comment 15

6 years ago
Created attachment 751792 [details] [diff] [review]
patch with  header information
Attachment #751788 - Attachment is obsolete: true
(Assignee)

Comment 17

6 years ago
That s awesome
https://hg.mozilla.org/mozilla-central/rev/a2bcdf9b6a2a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.