Closed Bug 91615 Opened 23 years ago Closed 21 years ago

Negative powers of numbers are not pretty-printed in msg viewer

Categories

(Core :: Networking, enhancement)

Product:
Core
Component:
Networking
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED WONTFIX

People

(Reporter: mdakin, Assigned: mdakin)

Details

Attachments

(3 files)

Composer sample of what I think the coding is supposed to be
367 bytes, text/html
Details
left side is what mozilla renders, right side (in parenthesis) is what I write during message composition
3.95 KB, image/gif
Details
First patch, this solves some of the issues, can anyone review this?
1.57 KB, patch
Details | Diff | Splinter Review 
Mozilla renders powers of numbers (like 10^3) like the real math representation
(A 10 and a little 3 on it) but it does not work for negative powers of numbers
like (10^-3)
How to reproduce:
 1.Open Mail-News window
 2.Choose new Message from menu
 3.Write 10^3 and 10^-3
 4.Save the mesage
 5.Check the mail from drafts folder, it doesnt show 10^-3 like it shows 10^3

Version Info:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.2+) Gecko/20010714
over to eiditor team. I don't know if it's a bug or a feature request!
Assignee: ducarroz → beppe
change qa contact since this is editor bug
QA Contact: sheelar → sujay
Marking NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: correctness
can you supply a testcase -- are you entering shift+6, or are you adding an entity?
A warning about the attachment I sent, The writings on the right side, in the
messages, "spaces" between "numbers" and "^" signs were writtenput deliberately
to show the error.
Mehmet has been very patient with me on this one. He sent a note explaining
exactly how t reproduce this. The key, I believe, is that he is using a Turkish
keyboard. This is the information that he sent me:

1. Open mozilla. (I am currently using  Mozilla/5.0 (Windows; U;      
Windows NT 5.0; en-US; rv:0.9.2) Gecko/20010628)                      
2. Open mail-news                                                     
3. Make a new mesage by clicking CTRL+M or  selecting "New message"   
from  FILE menu, now we have an empty message window                  
4. Write a random mail adress to "To:" edit box, since we wont send   
this to anyone. and write a random subject if you wish.               
5. Start writing test code by:                                        
  a.set the caret to main message area                                
  b.select 1 and 0 keys                                               
  c.select SHIFT+3 key (since this is a Turkish keyboard , this is the
"^" character, layout of english keyboards are different , so I       
suppose  you may press something else to produce ^ character          
  d.select 3 key                                                      
  e.now press enter, and go to newline                                
  f. repeat a,b,c steps                                               
  g. select - and 3 keys                                              
  (you can also write 103.14 too if you wish)                        
6. From file menu , select SAVE option (or click CTRL+S), now the     
message is saved into drafts folder of your local folders in mail     
window                                                                
7.Close the message window                                            
8.Now select "drafts" folder from your local folders on the left pane 
of mail-news window                                                   
9.click on the message you have just saved                            
10. And observe the difference

Entering the ^ from the English keyboard does not produce the superscript as he
explains.

I am reassigning this one to ftang
Assignee: beppe → ftang
I do not think the problem is my Turkish keyboard, I tried this on different
machines and copy-pasting "^" to the examples not typing it from keyboard. 
Can u send the example I have written to me, so we can test it whether its from
the keyboard layout or not.
The adress is: gbetul@ttnet.net.tr
This is not an internationalization bug. It looks like it is control by the 
Message Display Emoticons code. Try the following: go to 
Edit : Preference : Mail and Newsgroup : Message Display 
unchck the "Display emoticons as graphics"
and load that message again, it will turn off that display.
check the "Display emoticons as graphics"
it will turn it back. 
The code is in mozilla/netwerk/streamconv/converters/mozTXTToHTMLConv.cpp

rhp is no longer in the company, reassign to mscott.
Assignee: ftang → mscott
Well, I think I've found the guilty code in 

http://lxr.mozilla.org/mozilla/source/netwerk/streamconv/converters/mozTXTToHTMLConv.cpp

if you check it, it tries to get the "number" by finding the first non-digit
character after the '^' character. (can be seen on line 763)

But in our example like 10^-3 the first non digit character after the '^' is a
minus , so it just ignores the rest.

also if you write 10^3.14 it scans the first non digit after ^  and finds the
dot '.' char and adds 'sup' tag to '3', not '3.14'

Todo: With a very simple change i the code this can be fixed. program should get 
the real number after '^' or it should get all chars after '^' until it
encounters a word delimiting character like space or tab or enter. Fix is very
simple you can do it in five minutes , I do not have opportuninty to compile the
moz code so cannot submit a "real" fix.

751 if    // x^2 -> sup
...
...
763 // Find first non-digit
764 PRInt32 delimPos = 3;  // 3 = Position after first digit after "^"
765 for (; delimPos < aInLength &&
766 nsCRT::IsAsciiDigit(aInString[PRUint32(delimPos)]); delimPos++)
767 ;
768 // Note: (delimPos == text.Length()) could be true
769 
770 if (nsCRT::IsAsciiAlpha(aInString[PRUint32(delimPos)]))
771 {
772 MOZ_TIMER_STOP(mGlyphHitTimer);
773 return PR_FALSE;
774 }
775 776 outputHTML.Truncate();
777 outputHTML += text0;
778 outputHTML.AppendWithConversion("<sup class=\"moz-txt-sup\">");
779 780 aOutputString.Append(outputHTML);
781 aOutputString.Append(&aInString[2], delimPos - 2);
782 aOutputString.AppendWithConversion("</sup>");
783 784 glyphTextLen = delimPos /* - 1 + 1 */ ;
785 MOZ_TIMER_STOP(mGlyphHitTimer);
786 return PR_TRUE;
787 }


By the way, in the same file there is these lines, anyone aware of them?
(it says FIX ME!, but doesnt seem to be fixed yet. it makes a copy of entire 
buffer..)

1155 NS_IMETHODIMP
1156 mozTXTToHTMLConv::ScanHTML(const PRUnichar *text, PRUint32 whattodo,
1157                 PRUnichar **_retval)
1158 {
1159   NS_ENSURE_ARG(text);
1160 
1161   // FIX ME!!!
1162   nsString outString;
1163   nsString inString (text); // look at this nasty extra copy of the entire 
input buffer!
1164   outString.SetCapacity(PRUint32(inString.Length() * growthRate));
1165 
1166   ScanHTML(inString, whattodo, outString);
1167   *_retval = outString.ToNewUnicode();
1168   return *_retval ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
1169 }

1., assuming that you send in plain text, the "bug" is in the *viewer*, not the
composer.

2., most of this is not a bug. We fail to pretty-print in some cases, and I was
aware of that when writing it an accepted it. The groundrule was to prefer a
false negative over a false positive (i.e. better write it as-is in cases, where
it could be pretty-printed, than wrongly pretty-printing it). This is especially
true for the glyph-munger discussed here, because it actually *removes* chars
from the displayed body (namely "^").
In short, this is not a bug, but a low-priority enhancement suggestion. (With
one exception, see below.)

> also if you write 10^3.14 it scans the first non digit after ^  and finds the
> dot '.' char and adds 'sup' tag to '3', not '3.14'

That's unintended and the only real bug here, but not a very severe one either,
because the reader can probably guess, what went wrong and how it's supposed to
look.

From patch:
> + ItMatchesDelimited(aInString, aInLength, "^", 1, LT_ALPHA, LT_IGNORE) ||

That's not good. It will also match "3^abcdefgh", which is not intended. You may
want to change ItMatchesDelimited to consider ".-" as digits, but be very very
careful that you don't introduce unintended side-effects, because that
function/mode might be used elsewhere in the file.

>  // Find first non-digit
>      PRInt32 delimPos = 3;  // 3 = Position after first digit after "^"
> -    for (; delimPos < aInLength &&
> -         nsCRT::IsAsciiDigit(aInString[PRUint32(delimPos)]); delimPos++)
> -      ;
> +    // BUG 91615 - negative powers of numbers are not shown correctly in mail
> composition
> +	// Just a quick fix, previous version scanned the string after '^' until >
finding a 
> +	// Non-digit number, this one scans until it encounters a white-space
> +	for (; delimPos < aInLength &&
> +           (nsCRT::IsAsciiSpace(aInString[PRUint32(delimPos)])==PR_FALSE); >
> delimPos++)
> +	  ;
>      // Note: (delimPos == text.Length()) could be true

Same here. Much too broad. (BTW, you didn't adapt the comments either.)

Either scan for any char in "1234567890.-" only or forget about the case "3^34.346".
Severity: normal → enhancement
Component: Composition → Networking
OS: Windows 2000 → All
Product: MailNews → Browser
Hardware: PC → All
Summary: negative powers of numbers are not shown correctly in mail composition → Negative powers of numbers are not pretty-printed in msg viewer
> ".-"

BTW: German uses "," instead of ".", not sure about other European languages. Do
we want to include that too?

Also, you *have* to make sure that ".,-" are directly followed by a digit.,
otherwise the "." in "They sold at 10^12. " will be superscripted, which is wrong.

You see what this leads to? I tend towards WONTFIX, unless somebody can convince
me that he has a good patch that fixes the intended cases *only*.
I think now I can write a better fix than previous, 
I can take this if u wish.
I think now I can write a better fix than previous, 
I can take this if u wish.
-> Mehmet per his request.
Assignee: mscott → mdakin
removing myself from the cc list
I still see this bug in 1.5 beta.

I agree this is a pretty tricky problem, since one might want to have an
arbitrary expression in the exponent, like x^e or x^(y+z). Also you can't assume
the . (or ,) will come immediately -- x^-0.3 for example. But I think if we only
try to implement this for numbers, it can be done. I'd suggest the following
algorithm:

Read from ^ to the next whitespace (or end of text). If what you read has no
digits, quit. If it does have digits, exclude all non-digit characters from the
end. Now see if what you've got parses into a number. If so, make it a superscript.

This will correctly handle things like this:
x^-0.5.........

And not muck with things like this:
x^x
x^6a6

---
Also, just incase someone does a search on "negative exponents" I'm sticking
that text in here.
This would trigger too often. False positives are bad.
Hi, 

I am supposed to fix this problem but here's  what I suggest. After thinking
about it, Showing powers of numbers pretty becomes quite a complex problem and
requires a little state machine of its own.

Since we dont want to make the code overly complex, speed up mail composition
and reduce fottprint, I suggest we drop this kind of pretty printing all.
What you say about this? as far as I remember the code responsible to create a
pretty printed text is a little messy and you can see some buffer cloning codes
with "FIX ME" comments on them. The code looks better than I looked 3 years ago
but still needs atttention I guess.

http://lxr.mozilla.org/mozilla/source/netwerk/streamconv/converters/mozTXTToHTMLConv.cpp

> I suggest we drop this kind of pretty printing all.

It's 70 lines of code (with lots of whitespace) and usually performs only 3
integer comparions, if not triggered.

Or do you want to get rid of the smiley recognition as well? Because that's
really a mess (and could need an update), but the feature is a boon to users.

> as far as I remember the code responsible to create a pretty printed text
> is a little messy

But I don't see how the +/- and power-of code is messy. Line 882-946 currently,
starting with "if (text0 == '+' || text1 == '+')". If you can write it better
(same or better coverage of cases, same or better speed, clearer/less code),
please submit a patch.

> you can see some buffer cloning codes with "FIX ME" comments on them.

Some parts got really messy when certain people got their hands in and converted
it to using cstrings (instead of letting me fix it properly) and put these
unqualified FIXME in.
IF you really do change the code, please first apply the patch in bug 116242, so
we don't get conflicts.

If you do not plan to work on it, please just close it as WONTFIX. I don't see
the point either.

(And I wouldn't mind all that much either, if the +/- and ^ code, the 70 lines
mentioned above, got removed.)
>Or do you want to get rid of the smiley recognition as well? Because that's
>really a mess (and could need an update), but the feature is a boon to users.

I meant smiley and number code together.

>But I don't see how the +/- and power-of code is messy. Line 882-946 currently,
>starting with "if (text0 == '+' || text1 == '+')". If you can write it better
>(same or better coverage of cases, same or better speed, clearer/less code),
>please submit a patch.

I admit overall code looks better now, but I dont see any simpler and cleaner
way to  implement this correctly, things get trickier when you go into details.
there are some very stupid cases like: 10^-2*3 ,  2^.5 ,  5^(3A) , 2^1.3e10 etc.
we are not writing a mathematical editing program!

Now, I'm marking this bug as wontfix, since I dont have time to work on this.

Ben, It would be very nice if you can clean out (cut out) this formatting code,
I really dont think we need such stuff. 

I think we should also abandon smiley stuuf too. but as you said, that might be
depend on users.

->Wontfix
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WONTFIX
> I really dont think we need such stuff.

I'm a bit confused. First, you wanted to have negative numbers supported and now
you don't think we need positive ones either?
We don't need this. I just thought that it (what I implemented) was nice to have.
We do need the smileys feature, though.

verified wontfix.
Status: RESOLVED → VERIFIED
>I'm a bit confused. First, you wanted to have negative numbers supported and now
>you don't think we need positive ones either?

Yes. Because it is not working correctly for the situations I mentioned before.
2^.5 ,  5^(3A) , 2^1.3e10, 10^3.121, 10^+3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: