[FIXr]URI fixup should work with UTF8 strings

RESOLVED FIXED

Status

()

RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
Patch coming up.  This makes the code a little saner in a few places...
(Assignee)

Comment 1

16 years ago
Created attachment 126795 [details] [diff] [review]
Proposed patch
(Assignee)

Comment 2

16 years ago
Comment on attachment 126795 [details] [diff] [review]
Proposed patch

smontagu has tested this and it does not regress bug 58866
Attachment #126795 - Flags: superreview?(darin)
Attachment #126795 - Flags: review?(adamlock)

Comment 3

16 years ago
Comment on attachment 126795 [details] [diff] [review]
Proposed patch

this looks ok to me.  sr=darin with the following caveat...

as for the PossiblyByteExpandedFileName code... think about it this way.  if
native charset code were zero padded, it would still look like valid UTF-16. 
moreover, a conversion of such to UTF-8 would be well defined.	as such, the
conversion back from UTF-8 to UTF-16 could very well reintroduce characters
that are possibly byte expanded, so i think you should do away with your XXX
comment unless someone can confirm that we never see incorrectly formated
strings like this.
Attachment #126795 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 4

16 years ago
I had simon test, and we do see incorrect strings like that.

I'd like to change the XXX comment to say that the code _should_ be removed,
once bug xxxxx is fixed.  And I guess I should file that bug on intl?  Simon?

Comment 5

16 years ago
Comment on attachment 126795 [details] [diff] [review]
Proposed patch

Aside from the xxx comment, it looks okay to me too.

r=adamlock
Attachment #126795 - Flags: review?(adamlock) → review+
(Assignee)

Updated

16 years ago
Summary: [FIX]URI fixup should work with UTF8 strings → [FIXr]URI fixup should work with UTF8 strings
(Assignee)

Comment 6

16 years ago
The XXX comment I checked in:

        // XXXbz nsICmdLineService doesn't hand back unicode, so in some cases
        // what we have is actually a "utf8" version of a "utf16" string that's
        // actually byte-expanded native-encoding data.  Someone upstream needs
        // to stop using AssignWithConversion and do things correctly.  See bug
        // 58866 for what happens if we remove this
        // PossiblyByteExpandedFileName check.

jshin?  smontagu?  Any ideas on where the perfidious non-converting code lives?
(Assignee)

Comment 7

16 years ago
Fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 8

16 years ago
This appears to have killed one of the camino builds:

src/browser/BrowserWindowController.mm: In function `-[BrowserWindowController
goToLocationFromToolbarURLField:]':
src/browser/BrowserWindowController.mm:1175: no matching function for call to
`nsIURIFixup::CreateFixupURI(nsAutoString&, int, nsGetterAddRefs<nsIURI>)'
/builds/tinderbox/SeaMonkey/Darwin_6.6_Depend/mozilla/dist/include/docshell/nsIURIFixup.h:73:
candidates are: virtual nsresult nsIURIFixup::CreateFixupURI(const nsACString&,
unsigned int, nsIURI**)
(Assignee)

Comment 9

16 years ago
Camino attempted fix checked in.
You need to log in before you can comment on or make changes to this bug.