228904 - Preference panel(s): 'onload' event handling cleanup

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Serge Gautherie (:sgautherie)]

Spun off of bug 226958 comment 42.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <pref.xul ++> [Checked in: see Comment 8 and 9]

Comment 2

[Hermann Schwab]

From LXR:

 18  * Contributor(s):
 19  *   Ben "Count XULula" Goodger <[e-mail]>

 27  *  =>> CHANGES MUST BE REVIEWED BY [e-mail]!! <<=

Is this address (Ben Goodger) still correct?

Comment 3

[Serge Gautherie (:sgautherie)]

I guess it's now 'ben#bengoodger.com', from
<http://www.mozilla.org/hacking/reviewers.html>.
But this has to be confirmed...

Comment 4

[Serge Gautherie (:sgautherie)]

ben#bengoodger.com:

> 19  *   Ben "Count XULula" Goodger <[e-mail]>

Do you want this line updated ?

> 27  *  =>> CHANGES MUST BE REVIEWED BY [e-mail]!! <<=

Do you want this line updated/deleted ?
Do you want to be the reviewer ?

Comment 5

[[e-mail]]

Comment on attachment 137677 [details] [diff] [review]
(Av1) <pref.xul ++>
[Checked in: see Comment 8 and 9]

r=me although some of the indentation looks odd, is there a reason for that?

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 137677 [details] [diff] [review]
(Av1) <pref.xul ++>
[Checked in: see Comment 8 and 9]


Re comment 5:

I used a 2-char indentation:

|onAccept:| is lined up with |onload:| and |openBranch:|;
the 2 |return true;| looks odd because the existing code is "more" indented
than "needed".

I could change this if requested...

Comment 7

[Alec Flett]

Comment on attachment 137677 [details] [diff] [review]
(Av1) <pref.xul ++>
[Checked in: see Comment 8 and 9]

sr=alecf

Comment 8

[[e-mail]]

Fix checked in.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 137677 [details] [diff] [review]
(Av1) <pref.xul ++>
[Checked in: see Comment 8 and 9]


(for the record)
The |<separator/>| removal was done in bug 223295 in the meantime.

Comment 10

[Frank Wein [:mcsmurf]]

Might Bug 236467 have been caused by this bug (the regression window would fit)? 
ccing neil, cause he might know better...

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> Might Bug 236467 have been caused by this bug (the regression window would fit)? 

(I doubt it, but:)

> ccing neil, cause he might know better...

See also bug 236467 comment 11.

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> Might Bug 236467 have been caused by this bug (the regression window would fit)? 

It seems unrelated, per bug 236467 comment 19:
{{
this bug is not limited to the preferences window
}}