57006 - Incorrect hardcoded number in nsCaret.cpp causes bad cursor widths

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Mike Kaply [:mkaply]]

In nscaret.cpp:

http://lxr.mozilla.org/seamonkey/source/layout/base/src/nsCaret.cpp#745

  mCaretTwipsWidth  = 15 * mCaretPixelsWidth;//uhhhh...

Is incorrect.

15 should not be hardcoded.

That 15 is actually the DevUnitstoAppUnits value and should be queried from the 
Device Context.

Comment 1

[karnaze (gone)]

Reassigning to mjudge.

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

Comment 3

[Mike Kaply [:mkaply]]

Should I have set dx to nsnull at the end of that to free the pointer?

Comment 4

[Scott Collins]

I suggest



  presContext->GetDeviceContext(getter_AddRefs(dx));

  if ( dx )

    dx->GetDevUnitsToTwips(tDevUnitsToTwips);



for the middle hunk.  I think it's clearer.

Comment 5

[rubydoo123]

moving to mozilla0.9

Comment 6

[Mike Kaply [:mkaply]]

Can I get an r= based on scc comments?

Comment 7

[Mike Kaply [:mkaply]]

OK, this one is getting too old.

We need this fix.

Copying brendan.

Comment 8

[Brendan Eich [:brendan]]

Can you attach a patch based on scc's comments?  Then buster or waterson could 
give an sr= (mail 'em, cc: reviewers -- I didn't see a posting from you there 
on this bug).

/be

Comment 9

[Mike Kaply [:mkaply]]

Attachment: New diff - takes into account scc's suggestions

Comment 10

[Brendan Eich [:brendan]]

mjudge, can you add an r= to scc's contingent one?  Thanks.

/be

Comment 11

[mjudge]

hmm sfraser should really get this one. i will CC him only since a fix is ready.

r= mjudge. code looks clean. i have not added this and tested it myself yet 
(build still working) but it certainly looks good. i will update when i have 
actually run with it, but you can check it in before then on r=mjudge.

Comment 12

[Simon Fraser [no longer active]]

sr=sfraser on the patch

Comment 13

[Mike Kaply [:mkaply]]

Fix checked in.

Comment 14

[Mike Kaply [:mkaply]]

This is working.