I was looking at nsJISx4501LineBreaker, and it's a bit of a mess. There are a bunch of loops that have the first iteration of the loop manually pulled out. I have a patch to fix those, and fix the places where tabs are used in the file, and a few other little bits of cleanup. It probably needs a bit more testing than I've given it, and it's also worth double-checking that the stuff I pushed back into the loop really is equivalent.

Actually, now I see that my change to nsJISx4501LineBreaker::Prev wasn't quite keeping things the same (though it would be nice to know if the difference in |oPrev| output was intentional).

Actually, I just realized that the standard is really "JIS X 4051", so the changes to comments should not be there (and there should probably be others in the reverse direction).

And while I'm on the topic, the parameters to the constructor are unused, and should probably be eliminated, along with the whole concept of a line breaker factory (which exists only to pass those parameters, I think).

While you're at it, you may also consider renaming *4501* to *4051* risking the loss of the file change history. BTW, the parameters to the constructor arre not used now, but when language/script-dependent line breakers are implemented, they'd be of use. See bug 203016.

The files can be copied in the cvs repository in order to rename them. These are the changes that can happen without renaming files, though.

I've also been wondering whether we should rewrite the existing linebreaking code based on UAX #14. Doing so would handle a lot of cases better -- for example, hyphens (which are currently handled differently for CJK and non-CJK, and I think wrong in both cases). Doing this would mean that the nsTextFrame ASCII versions would have to call the linebreaker rather than considering only spaces. But I should probably file another bug about that...

I'll take a look at the patch. As for UAX #14, see bug 56652 comment #18. See also bugs listed in bug 206152 for other line-breaker issues. There may be other bugs I didn't find.

Comment on attachment 129783 [details] [diff] [review] fix spelling mistakes r=jshin

Comment on attachment 129783 [details] [diff] [review] fix spelling mistakes > nsLWBreakerFImp::GetBreaker Could we change the code at the end of this function to not use 'NULL' and to use NS_ADDREF instead of calling AddRef() manually? sr=bzbarsky with that.

I'm actually going to put that in the next patch (actually get rid of the null checks entirely, I think), since I'd rather not check in substantive changes with spelling corrections.

This is the previous real patch, merged with the spelling fixes, and with the changes suggested in comment 3, comment 11, and comment 12.

Comment on attachment 131581 [details] [diff] [review] patch (diff -uw, for review, i.e., without whitespace changes) sr=bzbarsky, but yes, the oPrev thing looks wrong...

Comment on attachment 131581 [details] [diff] [review] patch (diff -uw, for review, i.e., without whitespace changes) r=jshin

Comment on attachment 131581 [details] [diff] [review] patch (diff -uw, for review, i.e., without whitespace changes) Patch checked in to trunk, 2003-09-23 00:30 -0700.

dbaron: Why is this bug still opened?

Closing this bug. Please feel free to open if something else is required.