Closed Bug 342133 Opened 15 years ago Closed 13 years ago

xmlhttprequest.send doesn't work in nativeuconv builds

Categories

(Core :: Internationalization, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: timeless, Assigned: glandium)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

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).
Attachment #226312 - Flags: superreview?(bzbarsky)
Attachment #226312 - Flags: review?(smontagu)
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.
Attachment #226312 - Flags: review?(smontagu) → review-
Er...  This should work.  If the problem is that the scriptable converter stream is broken, we should just fix it, no?
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.
> As far as I understood it's simply not built

Er... we should change that, imo.
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.
Attachment #226640 - Flags: review?(smontagu)
your patch seams like the right thing to do.
Attachment #226312 - Flags: superreview?(bzbarsky) → superreview-
(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
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).
ok, so... how is this not a duplicate of bug 333261?
It's not (except it now has more information)...
*** Bug 333261 has been marked as a duplicate of this bug. ***
Attachment #226640 - Attachment description: untested → tested with addon https://bugzilla.mozilla.org/attachment.cgi?id=227063
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 :)
Attachment #227063 - Flags: review?(smontagu)
Note xmlhttprequest works fine as currently is if you apply patch from bug #333261, which, I insist, is not a duplicate.
(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? ;) 
*** Bug 333261 has been marked as a duplicate of this bug. ***
Attachment #226640 - Flags: review?(smontagu) → review+
Comment on attachment 227063 [details] [diff] [review]
Addon for 226640: untested

r=me with the indentation fixed.
Attachment #227063 - Flags: review?(smontagu) → review+
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)
Attachment #226640 - Attachment is obsolete: true
Update: it doesn't work anymore on trunk, despite patch from bug 333261 being applied.
This fixes the issue for me
Attachment #227063 - Attachment is obsolete: true
Attachment #309539 - Flags: review?(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.
Attachment #309539 - Flags: review?(smontagu) → review+
Attachment #309539 - Flags: approval1.9?
Comment on attachment 309539 [details] [diff] [review]
fix for SetOutputErrorBehavior against trunk

a1.9=beltzner
Attachment #309539 - Flags: approval1.9? → approval1.9+
Assignee: timeless → mh+mozilla
Keywords: checkin-needed
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
Status: NEW → RESOLVED
Closed: 13 years ago
Component: XML → Internationalization
QA Contact: xml → i18n
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
You need to log in before you can comment on or make changes to this bug.