417489 - XML Parse Error is messy in RTL

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Behnam Esfahbod [:zwnj]]

Happens in Fx2.0.  Should check Fx3.0

Comment 1

[(no longer active)]

Attachment: Test case

This can be any invalid XML file.  I'm uploading this just as a quick way to test the problem.

Comment 2

[Behnam Esfahbod [:zwnj]]

Thanks Ehsan.

BTW I got the first error trying to see a .svgz file, which the mime type in http header was svg+xml.  I'm not sure where the problem is for this case.  The content wasn't text at all.  Is it really a server-side problem, or we should handle such cases?

Comment 3

[(no longer active)]

Attachment: Firefox 2 screenshot

Comment 4

[(no longer active)]

Attachment: Firefox 3 screenshot

A nightly Firefox 3 build makes the error appear RTL, so the problem is specific to Firefox 2.

Comment 5

[Behnam Esfahbod [:zwnj]]

Good job man.  So I think we cannot do anything about it.

Any idea, dup, etc about the wrong mime-type problem?

Comment 6

[(no longer active)]

Moving to Core:XML, since it has nothing to do with the Persian l10n.  Also, setting the dependency on Persian Fx2 tracker.

Comment 7

[(no longer active)]

(In reply to comment #5)
> Good job man.  So I think we cannot do anything about it.
> 
> Any idea, dup, etc about the wrong mime-type problem?

I'm still looking through this, but I doubt that it's unsolvable.  Whether or not the appropriate module owners are willing to take patches for the branch to fix this in the next dot release is another issue though.

Comment 8

[(no longer active)]

Comment on attachment 303285 [details]
Firefox 3 screenshot

Oops, Firefox 3 is definitely affected by this problem as well.  I made a mistake of testing this by setting bidi.direction to 2, which caused the #document node to have the rtl direction, thus pretending that the error page is RTL.

One solution can be to modify nsXMLContentSink to add a doctype on error pages to load chrome://global/locale/global.dtd, and use &locale.dir; as the dir attribute of the <parsererror> element.  Another possible solution would be to modify it to add the chrome://global/locale/intl.css stylesheet and handle this element in intl.css.

I'm not sure which solution is more viable...

Comment 9

[(no longer active)]

A related bug: bug 350597

Comment 10

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

Comment 11

[(no longer active)]

Attachment: Add intl.css to the XML error document

The fix to this bug is two-fold.  The attached patch enabled usage of intl.css in the XML error document.  RTL locales need to reverse the parsererror element in their intl.css files as well as I did for fa in this patch:

<http://hg.mozilla.org/l10n-central/fa/rev/28a679c4e43d>

Comment 12

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/7b44b97ec410

The Litmus test can use attachment 303278 [details] as the test case.

Comment 13

[(no longer active)]

In order to use this fix for he and ar, you need to do something like this: <http://hg.mozilla.org/l10n-central/fa/rev/28a679c4e43d>

Comment 14

[(no longer active)]

Comment on attachment 359979 [details] [diff] [review]
Add intl.css to the XML error document

Low-risk patch to fix an issue for RTL locales; nice to have for 1.9.1.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 359979 [details] [diff] [review]
Add intl.css to the XML error document

a191=beltzner

Can we get a Litmus testcase added as well?

Comment 16

[(no longer active)]

(In reply to comment #15)
> (From update of attachment 359979 [details] [diff] [review])
> a191=beltzner
> 
> Can we get a Litmus testcase added as well?

We're in the process of adding Litmus tests for many of the RTL bugs including this one.

Comment 17

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7d61a0567863