Last Comment Bug 15089 - XML Parser errors are not localizable.
: XML Parser errors are not localizable.
Status: VERIFIED FIXED
[fixinhand]
: helpwanted, l12y
Product: Core
Classification: Components
Component: XML (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla0.9.4
Assigned To: Heikki Toivonen (remove -bugzilla when emailing directly)
: Chris Petersen
: Andrew Overholt [:overholt]
Mentors:
: 80959 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 1999-09-28 11:41 PDT by Nisheeth Ranjan
Modified: 2001-09-05 17:18 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix, might need a little polish (11.52 KB, patch)
2001-08-14 17:38 PDT, Heikki Toivonen (remove -bugzilla when emailing directly)
no flags Details | Diff | Splinter Review
Proposed fix 1 (18.51 KB, patch)
2001-08-15 14:10 PDT, Heikki Toivonen (remove -bugzilla when emailing directly)
no flags Details | Diff | Splinter Review
Fix 1 with different cvs options (cvs -uwN) (14.17 KB, patch)
2001-08-15 16:24 PDT, Heikki Toivonen (remove -bugzilla when emailing directly)
no flags Details | Diff | Splinter Review
Proposed fix 2 (18.27 KB, patch)
2001-08-16 16:52 PDT, Heikki Toivonen (remove -bugzilla when emailing directly)
no flags Details | Diff | Splinter Review

Description Nisheeth Ranjan 1999-09-28 11:41:43 PDT
I'm keeping this on my plate for post-beta work.  CCing you guys to keep you in
the loop.
Comment 1 tao 1999-09-28 11:48:59 PDT
CC Erik.
Comment 2 Frank Tang 1999-10-04 11:55:59 PDT
nisheeth, can you add Jim Clark to the CC list ?
Comment 3 Nisheeth Ranjan 1999-10-05 11:32:59 PDT
James, I'm adding you here to keep you in the loop.  Any suggestions regarding
this bug are most welcome.  Thanks.
Comment 4 James Clark 1999-10-05 21:01:59 PDT
I've no idea how you normally handle error message localization.  Take the error
code returned by XML_GetErrorCode() (which will always be a member of the
XML_Error enum) and do whatever you usually do to generate a localized message
from an error code.
Comment 5 Nisheeth Ranjan 2000-04-04 14:36:16 PDT
Moving bugs out by one milestone...
Comment 6 tao 2000-04-13 16:01:44 PDT
Hi, Nisheeth:


Will end users see the error messages generated from expat? 

Since expat is primary C code and can't get access to xpcom objects such as
chrome registry, nsILocale, and stringBundle, it seems to me the best way to 
solve this problem is to have expat proprogate the error code up to the caller
in C++ or JS and use the err code as the key to access the error messsage in 
the properties files.

How does this sound? Let me know!

It seems to me 
if the users are going to see the messages
Comment 7 Nisheeth Ranjan 2000-04-21 18:19:22 PDT
Tao, yes, end users will see the expat error messages if they try to load a 
malformed XML file.  The error codes are already propagated from expat to 
nsExpatTokenizer.  Could you point me to some code that uses properties files 
for localization?  If the property files can be accessed and manipulated through  
objects that are accessible from the component manager, then I can put the error 
message localization code in nsExpatTokenizer.  Currently, the expat tokenizer 
generates tokens for displaying the error message and always uses English error 
messages.
Comment 8 tao 2000-04-21 18:53:55 PDT
Here is the idl of the stringBundle:
  http://lxr.mozilla.org/mozilla/source/intl/strres/public/nsIStringBundle.idl

And, its usage in Wallet:

  http://lxr.mozilla.org/mozilla/source/extensions/wallet/src/wallet.cpp#707


Note that the GetStringFromID() methods passed the resulting strings as 
PRUnichar*. Please do not convert it to CString, otherwise, non-ascii texts
will be truncated.



Thanks
Comment 9 ekrock's old account (dead) 2000-05-05 18:55:13 PDT
FYI: This is a "nice to have" but not required for FCS. Reasoning: if XML
content developers and XML app developers are doing their jobs correctly, users
should never be presented with invalid/error-generating XML. As a result, these
error messages should (usually) be seen only by XML developers during
development, not by end users. (Obviously developers will make mistakes, but
then they're making themselves and their site/app look bad and have an incentive
to fix it. And developers outside the U.S. are, sadly, already used to dealing
with English-language error messages.) As a result, it's not the end of the
world if these error messages aren't localized for FCS. Nisheeth--you're free to
mark FUTURE if you have other higher-priority work and are strapped for time.
Comment 10 Nisheeth Ranjan 2000-05-23 20:51:35 PDT
Marking future based on Eric's analysis.  Will keep it in mind for work that 
Heikki might do if he comes on board on time.  Thanks, Eric, for that analysis.
Comment 11 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-06-07 09:37:09 PDT
*** Bug 80959 has been marked as a duplicate of this bug. ***
Comment 12 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-06-07 09:38:57 PDT
Taking, but if somebody has the cycles feel free to take this from me.
Comment 13 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-14 17:38:16 PDT
Created attachment 45857 [details] [diff] [review]
Fix, might need a little polish
Comment 14 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-15 14:10:27 PDT
Created attachment 45962 [details] [diff] [review]
Proposed fix 1
Comment 15 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-15 14:15:53 PDT
Harish, can you review?

I am not so sure about where to place the string properties; I put them in
communicator/locale/layout. Also, I am still open to renaming nsParserMsgUtils
to something else, if you can think of a better one. The class is made so that
if we later want to make localized messages for the HTML parser, we don't need
to change any code, just add new properties files and edit makefiles.

I still need to do some Mac build changes as well, but this can be reviewed for
other platforms.

nsBranch, moving to 0.9.4 and upping priority.
Comment 16 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-15 16:24:33 PDT
Created attachment 45990 [details] [diff] [review]
Fix 1 with different cvs options (cvs -uwN)
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2001-08-16 11:00:45 PDT
sr=jst
Comment 18 harishd 2001-08-16 13:58:25 PDT
+  nsCOMPtr<nsIIOService> pNetService(do_GetService(kIOServiceCID, &rv));
+  if (NS_SUCCEEDED(rv) && pNetService) {

Checking for NS_SUCCEEDED(rv) should be sufficient...right? 

+    rv = pNetService->NewURI(aPropFileName, nsnull, getter_AddRefs(uri));
+    if (NS_SUCCEEDED(rv) && uri) {

Same as above. Also, are you sure about propagating all failure messages? Or
should we just worry about NS_ERROR_OUT_OF_MEMORY?


Comment 19 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-16 16:50:28 PDT
You are right that NS_SUCCEEDED() is enough for do_GetService() and NewURI(), I
fixed that. Regarding the error returns, we are ignoring the return values for
CreateErrorText() and PushXMLErrorTokens() so we don't propagate stuff up from
there.

The most likely reason why the GetLocalizedStringBy*() functions would fail is
missing localization file or something like that. With the patch as is, we would
not update the content area which looks bad. So as a compromise, I am going to
ignore the return value of those functions in PushXMLErrorTokens() so that we at
least get a page giving the user some indication an error happened even though
some information is missing. jst ok'd this change.

I'll attach the latest patch.
Comment 20 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-16 16:52:40 PDT
Created attachment 46149 [details] [diff] [review]
Proposed fix 2
Comment 21 Frank Tang 2001-08-17 16:06:25 PDT
r=ftang for "localization, location of string resource etc."
Comment 22 Heikki Toivonen (remove -bugzilla when emailing directly) 2001-08-17 16:33:06 PDT
Fixed.
Comment 23 Chris Petersen 2001-09-05 17:18:53 PDT
Since I 'm not exactly sure how to test this, I'm marking verified based on the
last comments


Note You need to log in before you can comment on or make changes to this bug.