446027 - Code misbehavior between <pref-colors.xul> and <pref-editing.xul>

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.2pre) Gecko/2008071502 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

1. Open Preferences.
2. ("first") Load "Composer > New Page Settings".
3. (optional) Load any other panel.
4. (then) Load "Appearance > Colors"
4r.
{{
Warning: Empty string passed to getElementById().
}}

I debugged it with Venkman:
<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/pref/pref-colors.js&rev=1.1&mark=47,52#38>
retrieves
<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/editor/ui/composer/content/pref-editing.xul&rev=1.41&mark=165-167#160>
instead of
<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/common/pref/pref-colors.xul&rev=1.60&mark=91-92#87>
:-/

That's very weird/scary if some panel code ends up manipulating some other panel elements !

NB:
These codes look good; I would guess there is something wrong in the underlying preference panel mechanism.
This case by itself is "minor", but I fear a potential "major" underlying issue.

Comment 1

[Ian Neal]

It is is because they are overlays I suspect - one solution is to make sure all ids are unique across all pref panes. I do not know if there is another way yet.

Comment 2

[Karsten Düsterloh]

While we could (mis)use getElementsByAttribute to solve the issue, I think it's best for everyone's sanity to fix up the IDs. After all, IDs should be unique...

Comment 3

[Serge Gautherie (:sgautherie)]

Then, we need to add a prefix to every |id| on all panels !!?

Comment 4

[Karsten Düsterloh]

Well, most of the time the elements with IDs are closely tied to the panel's elements and as such aren't likely to clash - they just mustn't be too generic like "background"...

Comment 5

[Stefan [:stefanh]]

It would be really good if someone could check if there are more id duplicates. Or can we build in some kind of error handling somewhere (like "error: duplicate id's in xy.xul and yz.xul)?

Comment 6

[Karsten Düsterloh]

That's what automated tests are made for, probably a chrome test.

Comment 7

[Ian Neal]

Attachment: rename pref-colors ids patch v0.1 (Checked into trunk)

This patch:
* Renames various ids in pref-colors.xul to make them more likely to be unique
* Adjusts pref-colors.js to use new ids and fixes potential issue for when some prefs are locked

Comment 8

[Karsten Düsterloh]

I spun off the test into bug 446281.

Comment 9

[Ian Neal]

Comment on attachment 330429 [details] [diff] [review]
rename pref-colors ids patch v0.1 (Checked into trunk)

Checking into trunk
pref-colors.js;
new revision: 1.2; previous revision: 1.1
pref-colors.xul;
new revision: 1.61; previous revision: 1.60
done

Comment 10

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.2pre) Gecko/2008072102 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

V.Fixed