727929 - test_idcheck.xul > preferences.xul: doubled 'languageBundle' and 'regionBundle'

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P2)

Description

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329215161.1329219819.16122.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/02/14 02:26:01
{
Double id='languageBundle' detected in chrome://communicator/content/pref/preferences.xul:
  Tree 0:
    stringbundle languageBundle [0]
    prefpane spelling_pane [10]
    prefwindow prefDialog [27]
    #document undefined [0]
  Tree 1:
    stringbundle languageBundle [0]
    prefpane composing_messages_pane [23]
    prefwindow prefDialog [27]
    #document undefined [0]
18104 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: preferences.xul#languageBundle

Double id='regionBundle' detected in chrome://communicator/content/pref/preferences.xul:
  Tree 0:
    stringbundle regionBundle [1]
    prefpane spelling_pane [10]
    prefwindow prefDialog [27]
    #document undefined [0]
  Tree 1:
    stringbundle regionBundle [1]
    prefpane composing_messages_pane [23]
    prefwindow prefDialog [27]
    #document undefined [0]
18105 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: preferences.xul#regionBundle
}

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1329290562.1329294861.20808.gz
OS X 10.5 comm-aurora debug test mochitest-other on 2012/02/14 23:22:42
too.

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1329367905.1329371576.30324.gz
OS X 10.5 comm-beta debug test mochitest-other on 2012/02/15 20:51:45
is not affected.

***

http://mxr.mozilla.org/comm-central/search?string=languageBundle&case=1&find=%2Fsuite%2F.*%2Fpref.*\.xul%24
http://mxr.mozilla.org/comm-central/search?string=regionBundle&case=1&find=%2Fsuite%2F.*\.xul%24

Bug 527173 duplicated them.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

pref-appearance.xul is similar but lucky enough to not have duplicates right now:

     <stringbundleset id="prefAppearanceBundleset">
       <stringbundle id="langNamesBundle"
                     src="chrome://global/locale/languageNames.properties"/>
       <stringbundle id="regNamesBundle"
                     src="chrome://global/locale/regionNames.properties"/>
     </stringbundleset>

I suggest we prefix all 3x2 = 6 occurrences, e.g.

prefAppearanceLangNamesBundle
prefAppearanceRegNamesBundle

and introduce stringbundlesets for pref-composing_messages.xul and pref-spelling.xul while we're here.

Or maybe I know too less about the preferences window code and there's a chance to only have the stringbundles once, globally?

Comment 2

[Ian Neal]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #1)

> and introduce stringbundlesets for pref-composing_messages.xul and
> pref-spelling.xul while we're here.
Yes please.
> 
> Or maybe I know too less about the preferences window code and there's a
> chance to only have the stringbundles once, globally?
Yes, they can be loaded in preferences.xul and then just referenced from there.

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch [Checkin: Comments 5 and 11]

I decided to fix all pref-* stuff while we're here, but not change other IDs like bundle_prefutilities (used quite often and maybe also by add-ons).

Comment 4

[Ian Neal]

Comment on attachment 598644 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

>+++ b/suite/common/pref/preferences.xul
>+  <stringbundleset id="prefBundleset">
>+    <stringbundle id="bundle_prefutilities"
>+                  src="chrome://communicator/locale/pref/prefutilities.properties"/>
>+  </stringbundleset>
>+
>+  <stringbundleset id="langRegionNamesBundleset">
>+    <stringbundle id="languageNamesBundle"
>+                  src="chrome://global/locale/languageNames.properties"/>
>+    <stringbundle id="regionNamesBundle"
>+                  src="chrome://global/locale/regionNames.properties"/>
>+  </stringbundleset>
I don't think these need to be in separate bundlesets, so just have then in the one "prefBundleset"
r=me with that change.

I presume that test_idcheck.xul no longer fails with this patch applied?

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 598644 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

http://hg.mozilla.org/comm-central/rev/3b59be34ef07

(In reply to Ian Neal from comment #4)
> I presume that test_idcheck.xul no longer fails with this patch applied?

Have a little faith! :-)

Comment 6

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329715249.1329720463.9120.gz
Linux comm-central-trunk debug test mochitest-other on 2012/02/19 21:20:49

V.Fixed

Comment 7

[[e-mail]]

I wonder whether it's worth overlaying prefBundleset with stringbundles rather than having separate stringbundlesets inside the prefpanes? That way the ids would simply merge together in the same element.

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] from comment #7)
> I wonder whether it's worth (...)

No idea, give it a try (or ask Ian specifically)! I'm not opposed to it.

Comment 9

[Serge Gautherie (:sgautherie)]

Ping for approval.

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 598644 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

Safe, no l10n changes, baked on trunk, verified, fixes perma-orange -> a=me.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 598644 [details] [diff] [review]
patch [Checkin: Comments 5 and 11]

http://hg.mozilla.org/releases/comm-aurora/rev/d823d69dddab

Comment 12

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1331351814.1331355679.24663.gz
OS X 10.5 comm-aurora debug test mochitest-other on 2012/03/09 19:56:54

seamonkey2.9: verified.