XML Parser errors are not localizable.

VERIFIED FIXED in mozilla0.9.4

Status

()

Core
XML
P2
normal
VERIFIED FIXED
18 years ago
16 years ago

People

(Reporter: Nisheeth Ranjan, Assigned: Heikki Toivonen (remove -bugzilla when emailing directly))

Tracking

({helpwanted, l12y})

Trunk
mozilla0.9.4
helpwanted, l12y
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixinhand])

Attachments

(4 attachments)

(Reporter)

Description

18 years ago
I'm keeping this on my plate for post-beta work.  CCing you guys to keep you in
the loop.
(Reporter)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: M15

Comment 1

18 years ago
CC Erik.
(Reporter)

Updated

18 years ago
Whiteboard: HELP WANTED

Comment 2

18 years ago
nisheeth, can you add Jim Clark to the CC list ?
(Reporter)

Comment 3

18 years ago
James, I'm adding you here to keep you in the loop.  Any suggestions regarding
this bug are most welcome.  Thanks.

Comment 4

18 years ago
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.

Updated

18 years ago
Keywords: helpwanted
Whiteboard: HELP WANTED
(Reporter)

Comment 5

18 years ago
Moving bugs out by one milestone...
Target Milestone: M15 → M16

Comment 6

18 years ago
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
(Reporter)

Comment 7

18 years ago
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

18 years ago
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
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.
(Reporter)

Comment 10

18 years ago
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.
Target Milestone: M16 → Future

Updated

17 years ago
QA Contact: chrisd → petersen
*** Bug 80959 has been marked as a duplicate of this bug. ***
Taking, but if somebody has the cycles feel free to take this from me.
Assignee: nisheeth → heikki
Status: ASSIGNED → NEW
OS: Windows NT → All
Priority: P3 → P4
Summary: Localize the error strings returned by expat (our XML parser) → XML Parser errors are not localizable.
Target Milestone: Future → mozilla0.9.2
Keywords: l12y
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla1.0

Updated

16 years ago
Keywords: nsBranch
Created attachment 45857 [details] [diff] [review]
Fix, might need a little polish
Created attachment 45962 [details] [diff] [review]
Proposed fix 1
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.
Status: NEW → ASSIGNED
Priority: P4 → P2
Whiteboard: [fixinhand]
Target Milestone: mozilla1.0 → mozilla0.9.4
Created attachment 45990 [details] [diff] [review]
Fix 1 with different cvs options (cvs -uwN)
sr=jst

Comment 18

16 years ago
+  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?


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.
Created attachment 46149 [details] [diff] [review]
Proposed fix 2

Comment 21

16 years ago
r=ftang for "localization, location of string resource etc."
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 23

16 years ago
Since I 'm not exactly sure how to test this, I'm marking verified based on the
last comments

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.