Closed Bug 128530 Opened 22 years ago Closed 22 years ago

Use |nsISaveAsCharset::attr_EntityAfterCharsetConv| instead of |nsISaveAsCharset::attr_EntityBeforeCharsetConv| in GTK+/Xlib toolkits

Categories

(Core :: XUL, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.0

People

(Reporter: roland.mainz, Assigned: roland.mainz)

References

Details

Attachments

(1 file, 1 obsolete file)

[rbs wrote in bug 33498 comment #9]
> In the process, I noted that other platforms are using 
> nsISaveAsCharset::attr_EntityBeforeCharsetConv. I think what people want is
> nsISaveAsCharset::attr_EntityAfterCharsetConv. 
>
> The former will convert to entities _before_ doing the transliteration, 
> thereby possibly converting some glyphs that could be transliterated. The 
> latter will transliterate, and then the glyphs that couldn't be transliterated 
> will be converted to their entity representation if possible or to question 
> marks if not possible. Finally, just in case, I also made sure that the Win32 
> routines that expect the converted result are 0-length safe. Not sure about 
> other platforms here.

RFE: Do it!
Attached patch Patch for 2002-02-28-08-trunk (obsolete) — Splinter Review
Requesting r=/sr=/a= ...
Status: NEW → ASSIGNED
Keywords: patch, review
Target Milestone: --- → mozilla1.0
Depends on: 85417
No longer depends on: 85417
Depends on: 85417
Comment on attachment 72147 [details] [diff] [review]
Patch for 2002-02-28-08-trunk

r=rbs
Attachment #72147 - Flags: review+
rbs:
Do we have any testcase to see the "before patch <--> after patch"-effect ?
I would like to (strongly) recommend we have Frank Tang, the converter expert,
look at this.
>rbs:
>Do we have any testcase to see the "before patch <--> after patch"-effect ?

No. But you can easily make one that will exercise the code as per my above 
description. See transliterate.properties for a sample of characters that can be
transliterated. To test if the code 0-length safe, you could try some of the
empty characters, eg., <span>&zwsp;&InvisibleComma;...etc</span>  (while making
sure that you have no fonts with glyphs for them...).
I think rbs rbs is right, but I don't really think this change will have any
effect since probably no characters which can be covered by "ISO-8859-1" will
ever hit the code here unless there are no ISO-8859-1 font installed on the
system, which is very very very unlikely.

I think we should not check in this patch now since:
1. there are no obvious gain (no functionality gain or loss with or without this
patch)
and 2. there are risk (tiny but not zero) if we take this patch. 

I think there are no reason to take a zero gain tiny risk patch in this stage.
why don'w we save it till post m1.0?
Because it frees your mind, reduce bug counts, and allows concentrating fully on
other important things.
Just wondering - why are we using "ISO-8859-1" and not $LANG or $LC_CTYPE (which
ever one is the correct one) - and fall back to "ISO-8859-1" on demand ?
waterson asked similarly in bug 33498 comment #18, reply: bug 33498 comment #18
ftang:
Windows works with this since eternity... I think Windows and X11(=GTK,Xlib)
should do the same.

Unless you see major problems I would like to get sr= and a= for trunk, please
...
Requesting sr= ...
Roland: could you point out some specific cases for this?

Without a strong case for doing this I would recommend we avoid the risk
till after m1.0.
Brian Stell wrote:
> Roland: could you point out some specific cases for this?

Mhhh... not really... 

... rbs - do you have one ?
I just happen to see a checkin comment on a related issue:
http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1018051860&maxdate=1018052880&who=nhotta%25netscape.com

(The trivial patch for this bug has been sitting here with a r= all along, it is
not really the best use of my time to drag over such intelligible matters on and
on.)
Comment on attachment 72147 [details] [diff] [review]
Patch for 2002-02-28-08-trunk

rs=attinasi
Attachment #72147 - Flags: superreview+
Attachment #72147 - Attachment is obsolete: true
Checked into "trunk" by timeless
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1018746780&maxdate=1018747800&who=timeless%25mac.com)
...

----

rbs:
Should I keep this bug open and wait a while and then request a= for the
1.0-branch again (assuming noone files a bug again this... :) ?
Why bother that much for something that minor? Just close the bug and move on,
and the patch will eventually make its way to another Mozilla release.
OK, marking bug as FIXED...
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: