Open Bug 428456 Opened 16 years ago Updated 16 years ago

no strings for some onlineTooltipN

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

People

(Reporter: ajschult784, Unassigned)

References

()

Details

bug 223908 added different tooltips for different values of the network.proxy.type.  Mine is set to 5 and I get no tooltip.  This results in

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://communicator/content/utilityOverlay.js :: setOfflineUI :: line 220"  data: no]

this was suppressed until today (bug 415498, apparently)
The fault seems to be that http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/common/utilityOverlay.js&rev=1.111&mark=219-220#202 just retrieves a value from a pref (or not if it throws) and appends it to a string to be used as ID for retrieving the tooltip without checking for valid values.
Flags: blocking-seamonkey2.0a1?
What do you mean by a valid value? 5 isn't currently implemented in the Preferences dialog yet.
valid string ID values is what I mean. Of course, it's not the job of this code to say what pref values are valid, but it _is_ its job to only call valid string IDs.
> What do you mean by a valid value? 5 isn't currently implemented in the
> Preferences dialog yet.

That doesn't stop it from being the default value:
http://mxr.mozilla.org/seamonkey/source/modules/libpref/src/init/all.js#791

Practically speaking, I have no idea what 5 means...
Apparently, it's "Use system proxy settings", see bug 416274 - and a value 4 exists as well, according to their pref panel.
Actually we have a string for 4. (3 is obsolete, and means 0.)
For some reason, illegal values default to 1 (manual proxy settings).

Anyway, I guess we need a bug on adding UI for this new value.
Seems like that. And in this bug, we should at least catch the exception and ideally not even try to get a string when getting the pref failed and the var is not set.
I don't think getting the pref is likely to fail ;-)
(In reply to comment #8)
> I don't think getting the pref is likely to fail ;-)

In that case, we should remove the try/catch around it. Trying to add an undefined var to what we hand to .GetStringFromName() does not sound like a good idea in any case to me, and the code reads in a way that this is possible.
(In reply to comment #9)
> (In reply to comment #8)
> > I don't think getting the pref is likely to fail ;-)
> In that case, we should remove the try/catch around it. Trying to add an
> undefined var to what we hand to .GetStringFromName() does not sound like a
> good idea in any case to me, and the code reads in a way that this is possible.
Hmm, I wonder who reviewed that :-[
Component: XP Apps: GUI Features → UI Design
The actual error was fixed in an additional patch for bug 416274 - do we still want to fix the try/catch and/or make it safer against wrong/future pref values?
Depends on: 416274
Flags: blocking-seamonkey2.0a1?
It is not clear to me whether the fix is ought to be applied over TRUNK but it
seems not. I am getting as of now the following:

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE)
[nsIStringBundle.GetStringFromName]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"
 location: "JS frame :: chrome://communicator/content/utilityOverlay.js ::
setOfflineUI :: line 220"  data: no]
************************************************************


Build platform
target
i686-pc-linux-gnu

Build tools
Compiler        Version         Compiler flags
gcc     gcc version 4.3.1 (Gentoo 4.3.1-r1 p1.1)        -Wall -W -Wno-unused
-Wpointer-arith -Wcast-align -W -Wno-long-long -pedantic -fno-strict-aliasing
-pthread -pipe
c++     gcc version 4.3.1 (Gentoo 4.3.1-r1 p1.1)        -fno-rtti
-fno-exceptions -Wall -Wconversion -Wpointer-arith -Woverloaded-virtual -Wsynth
-Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-long-long
-pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe

Configure arguments
--disable-optimize '--enable-debug=-g3\ -O0\ -ggdb' --enable-debug-modules=all
--enable-debugger-info-modules --enable-detect-webshell-leaks --enable-svg
--enable-svg-renderer-libart --enable-image-decoders=all --with-qtdir=/usr/qt/3
--enable-application=suite --disable-freetype2 --enable-jprof
--enable-default-toolkit=cairo-gtk2 --enable-xft --disable-gssapi
What do you mean by trunk? We switched from CVS to Hg before the fix landed.
Whatever I have fetched from the old CVS then. :((((( Why isn't it shutdown or having a different password?
(In reply to comment #14)
> Whatever I have fetched from the old CVS then. :((((( Why isn't it shutdown or
> having a different password?

Neither cvs nor hg have passwords, we're OPEN source, you know. :)

That said, maybe we should display some kind of warnings when people try to build SeaMonkey or Thunderbird from cvs - but that's something for other bugs.

I still wonder about my questions in comment #11 though.
You need to log in before you can comment on or make changes to this bug.