161677 - Preferences > Appearance > Colors horked, JS error

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

1. launch Mozilla (trunk)
2. open Preferences
3. choose Appearance > Colors
4. change radio-choice "Always use the colors..." & "Use my chosen colors..."
5. click on OK

Expected result : applies changes and closes window
Actual result : does not apply change, does not close window, outputs error on
console :

JavaScript error:
chrome://communicator/content/pref/pref-navigator.js line 141:
document.getElementById("startupPage") has no properties

Tested on Win2K and Linux RH72 with trunk (fresh pull 2002-aug-08 06:00am
pacific time)

Comment 1

[timeless]

this makes preferences unusable in the average case

Comment 2

[timeless]

Attachment: patch (first half checked in)

Comment 3

[timeless]

ben: this patch makes the pref dialog handle failures in ok/cancel handlers.
we've run into this problem before and it's really really bad for the ok and
cancel buttons not to work. -- you've been notified as per the heading of the
file, however i do intend to check this in unless you respond quickly indicating
why this is much worse than risking users locked in a pref dialog in the event
that a cancel and an ok handler throw exceptions

Comment 4

[Alec Flett]

Comment on attachment 94490 [details] [diff] [review]
patch (first half checked in)

ok, I'll sr= the first file in that patch, but I refuse to let the 2nd one in -
if you're going to touch the file, fix the bug the right way!

Comment 5

[Mark Anderson]

Comment on attachment 94490 [details] [diff] [review]
patch (first half checked in)

r=FrodoB (ditto on AlecF's comment).  I don't know why the OK and Cancel
handlers would fail, but these checks aren't bad to have even if they couldn't.
:)

Comment 6

[timeless]

Comment on attachment 94490 [details] [diff] [review]
patch (first half checked in)

ok. I checked the first half in. caillon can now take some time to fix this bug

Comment 7

[timeless]

*** Bug 161733 has been marked as a duplicate of this bug. ***

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Yes, silly mistake.  I thought I had handled that already but I must have
reverted patches and went a different route.  *sigh*.

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: The Real Fix

Comment 10

[Alec Flett]

Comment on attachment 94517 [details] [diff] [review]
The Real Fix

sr=alecf

Comment 11

[timeless]

Comment on attachment 94517 [details] [diff] [review]
The Real Fix

drop 'us' from the comment for r=timeless

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in.

Comment 13

[John Morrison]

*** Bug 161785 has been marked as a duplicate of this bug. ***

Comment 14

[timeless]

*** Bug 161835 has been marked as a duplicate of this bug. ***

Comment 15

[Olav Vitters]

*** Bug 161875 has been marked as a duplicate of this bug. ***

Comment 16

[Alfonso Martinez]

*** Bug 161905 has been marked as a duplicate of this bug. ***

Comment 17

[Olav Vitters]

*** Bug 161933 has been marked as a duplicate of this bug. ***

Comment 18

[[e-mail]]

btw, this also appears to fix a long standing rfe whereby the current page data
is now saved before any of the ok handlers are called, allowing the ok handlers
to read the page data without worrying whether the page is current or not, in
which case can someone please confirm that in my rfe to remind me to update the
patch.

Comment 19

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.09.16.08 comm trunk builds.