Closed
Bug 211181
Opened 21 years ago
Closed 21 years ago
[FIXr]URI fixup should work with UTF8 strings
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: bzbarsky)
Details
Attachments
(1 file)
22.51 KB,
patch
|
adamlock
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
Patch coming up. This makes the code a little saner in a few places...
Assignee | ||
Comment 1•21 years ago
|
||
Assignee | ||
Comment 2•21 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•21 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•21 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 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•21 years ago
|
Summary: [FIX]URI fixup should work with UTF8 strings → [FIXr]URI fixup should work with UTF8 strings
Assignee | ||
Comment 6•21 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•21 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 8•21 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•21 years ago
|
||
Camino attempted fix checked in.
You need to log in
before you can comment on or make changes to this bug.
Description
•