283134 - rectangles around selected text

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Joseph Izso]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (X11; U; Familiar 0.72; en-US; rv:1.7a) Gecko/20040218 Minimo/0.1

Before and after selection Minimo adds "rectangles" (special characters, perhaps?).
First rectangle is selected, second is after selection.

Because this adds two characters in text, paragraphs are reformatted with
selection changes.

Reproducible: Always

Steps to Reproduce:
1. Open web apge
2. Select some text on page
Actual Results:  
There should appear rectangles and text should move

Expected Results:  
Only select text without adding some special characters to selection.

Comment 1

[Joseph Izso]

Attachment: screenshot of selected text with rectangles arround it

Comment 2

[Doug Turner (:dougt)]

*** Bug 316900 has been marked as a duplicate of this bug. ***

Comment 3

[Doug Turner (:dougt)]

Attachment: Patch v.1

This seams to work.

If only the if-statement would result in the same thing.  I find that every predicate evals to zero.

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 203554 [details] [diff] [review]
Patch v.1

Could you advise on the above patch?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 203554 [details] [diff] [review]
Patch v.1

I cannot accept this patch.
We should not use #ifdef on XP code(of course, we don't have another way, we should go).

Now, we don't know why |PaintAsciiText| doesn't work fine on WinCE yet.
We need to know truth.

Why is |text| broken on |PaintAsciiText|?
I think that |DrawString| breaks |text| on WinCE, right?

Isn't regression of bug 260663?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: proposal patch

I think that the |text| is broken by 

3400 rv = aRenderingContext.GetWidth(currenttext, currentlength,newWidth); //ADJUST FOR CHAR SPACING

I think that the WinCE's GetTextExtentPoint32 is set NULL to lpString[cbString].
> BOOL GetTextExtentPoint32( 
> HDC hdc, 
> LPCTSTR lpString, 
> int cbString, 
> LPSIZE lpSize
> );

Please check this patch. I cannot test this.

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 203599 [details] [diff] [review]
proposal patch

works for me.  Thank you!

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, don't we need sr for this?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Added the comment

Comment 10

[Doug Turner (:dougt)]

Comment on attachment 203607 [details] [diff] [review]
Added the comment

i might be able to hide this stuff inside windows ce shunt library (mozilla/build/wince/shunt)... In this way, we do not have to add more ifdefs into the gfx code.

Comment 11

[Doug Turner (:dougt)]

found the problem in the shunt layer....

http://lxr.mozilla.org/mozilla/source/build/wince/shunt/win32A.cpp#649

I am removing this mistake.

Thanks for your help Masayuki!