342133 - xmlhttprequest.send doesn't work in nativeuconv builds

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[timeless]

this is a regression from bug 337704

I'll file another bug about changing intl to make it clear that consumers should generally use the charsetconvertermanager (which xmlhttprequest did until the changes by peterv).

Comment 1

[timeless]

Attachment: use the converter manager

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 226312 [details] [diff] [review]
use the converter manager

>Index: mozilla/content/base/src/nsXMLHttpRequest.cpp
>===================================================================

>+      PRInt32 outLength = srcLength*6;

Use encoder->GetMaxLength

>+          rv = stringDataStream->SetData(output, outLength);

Use AdoptData.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Er...  This should work.  If the problem is that the scriptable converter stream is broken, we should just fix it, no?

Comment 4

[Peter Van der Beken [:peterv]]

As far as I understood it's simply not built, see http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/Makefile.in#81 Personally, I'd prefer to see this kind of functionality available from the intl module, instead of adding the code in content.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> As far as I understood it's simply not built

Er... we should change that, imo.

Comment 6

[timeless]

well erm. i'm not sure i have much of an opinion, but clearly the minimalizers decided they wanted to not build it. and i'm not sure how they feel about you dictating to them.

(personally, i'm pretty sure i want scriptable access to intl even in minimo, but i'm not driving this, everyone else is pushing me in conflicting directions.)

i just want calendar to work.

Comment 7

[timeless]

Attachment: tested with addon https://bugzilla.mozilla.org/attachment.cgi?id=227063

Comment 8

[Doug Turner (:dougt)]

your patch seams like the right thing to do.

Comment 9

[Oleg Romashin (:romaxa)]

(In reply to comment #7)
> Created an attachment (id=226640) [edit]
> untested
> 

Has been tested, unsuccessfully:

Output log:

open_uri_cb about:blank
 * IConvAdaptor - - GetMaxLength 16384 ( UTF-8 -> UCS-2 )
 * IConvAdaptor - - GetMaxLength 8436 ( UTF-8 -> UCS-2 )
WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0', file ../../dist/include/layout/nsPresContext.h, line 846
WARNING: GetDefaultCharsetForLocale: need to add multi locale support: file nsUNIXCharset.cpp, line 189
WARNING: refusing to set request header: file nsXMLHttpRequest.cpp, line 1686
WARNING: Uconv Error Behavior not support: file nsNativeUConvService.cpp, line 260
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file nsXMLHttpRequest.cpp, line 1541
JavaScript error: http://www.google.com/calendar/20060620230205doozercompiled_en.js, line 868: u has no properties
js_status_cb
++DOMWINDOW == 5
js_status_cb

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yeah.  IConvAdaptor has at least two bugs:

1)  The check in IConvAdaptor::SetOutputErrorBehavior is backwards.  I _swear_
    I've seen a bug on that...
2)  In addition to kOnError_Replace it can support kOnError_Signal (in fact,
    that's what it does if SetOutputErrorBehavior is never called).

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, so... how is this not a duplicate of bug 333261?

Comment 12

[Boris Zbarsky [:bzbarsky]]

It's not (except it now has more information)...

Comment 13

[timeless]

*** Bug 333261 has been marked as a duplicate of this bug. ***

Comment 14

[Oleg Romashin (:romaxa)]

Attachment: Addon for 226640: untested

Comment 15

[timeless]

Comment on attachment 227063 [details] [diff] [review]
Addon for 226640: untested

sorry about the tabs, we'll try not to include them. romaxa's editor sucks :)

Comment 16

[Mike Hommey [:glandium]]

Note xmlhttprequest works fine as currently is if you apply patch from bug #333261, which, I insist, is not a duplicate.

Comment 17

[Oleg Romashin (:romaxa)]

(In reply to comment #15)
> (From update of attachment 227063 [details] [diff] [review] [edit])
> sorry about the tabs, we'll try not to include them. romaxa's editor sucks :)
> 

You mean Vim - is sucks? ;) 

Comment 18

[timeless]

*** Bug 333261 has been marked as a duplicate of this bug. ***

Comment 19

[Simon Montagu :smontagu]

Comment on attachment 227063 [details] [diff] [review]
Addon for 226640: untested

r=me with the indentation fixed.

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 226640 [details] [diff] [review]
tested with addon https://bugzilla.mozilla.org/attachment.cgi?id=227063

(I checked in essentially the same patch in bug 333261)

Comment 21

[Mike Hommey [:glandium]]

Update: it doesn't work anymore on trunk, despite patch from bug 333261 being applied.

Comment 22

[Mike Hommey [:glandium]]

Attachment: fix for SetOutputErrorBehavior against trunk

This fixes the issue for me

Comment 23

[Simon Montagu :smontagu]

Comment on attachment 309539 [details] [diff] [review]
fix for SetOutputErrorBehavior against trunk

FWIW, I prefer attachment 227063 [details] [diff] [review], though that would now need merging to trunk as well as untabifying, but r=me either way. sr is not required, but I think approval1.9 is, even though this is not part of the default build.

Comment 24

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

Comment on attachment 309539 [details] [diff] [review]
fix for SetOutputErrorBehavior against trunk

a1.9=beltzner

Comment 25

[Reed Loden [:reed]]

Checking in intl/uconv/native/nsNativeUConvService.cpp;
/cvsroot/mozilla/intl/uconv/native/nsNativeUConvService.cpp,v  <--  nsNativeUConvService.cpp
new revision: 1.10; previous revision: 1.9
done