Bug 13820

Summary: Height of cursor should be same as height of text, not line-height
Product: WebKit Reporter: Sridhar Gurivireddy <just1gb>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ahmad.saleem792, ap, bfulgham, joone, rniwa, victor.singh1983, webkit-bug-importer, webkit, yurtaev.egor
Priority: P4 Keywords: InRadar
Version: 523.x (Safari 3)   
Hardware: All   
OS: All   
Attachments:
Description Flags
test case
none
patch for fixing the cursor height issue hyatt: review-

Description Sridhar Gurivireddy 2007-05-22 11:39:58 PDT
When CSS attribute line-height is specified for <textarea>, height of cursor shown is equal to the value of attribute specified.

Spec "http://www.w3.org/TR/REC-CSS1#line-height" does not say anything about cursor size.

Behaviour

Safari: Shows a big cursor.
IE: shows smaller cursor (equal to height of text, not line-height)

Firefox: Definitely buggy.. shows big cursor for first charecter, shows smaller cursor for other charecters in a line
Comment 1 Alexey Proskuryakov 2007-07-13 05:27:58 PDT
Created attachment 15500 [details]
test case
Comment 2 Alexey Proskuryakov 2007-07-13 05:32:50 PDT
Confirmed with r24182. The behavior is slightly different from what Tiger WebKit had, but this probably doesn't qualify it as a regression.
Comment 3 amit 2007-11-29 12:43:34 PST
Created attachment 17595 [details]
patch for fixing the cursor height issue 

It a simple patch that should fix the cursor height issue in the caretRect method of RenderText. Also I had to override caretRect in RenderTextControlInnerBlock to take care of the initial state of the cursor (before first text char is entered).

This is my first attempt to hack webkit code- so please let me know if I am not following the process correctly.
Comment 4 Dave Hyatt 2007-11-29 12:47:52 PST
Comment on attachment 17595 [details]
patch for fixing the cursor height issue 

On Mac, the height of the caret is the full height of the line and not just the height of the text.  I am not sure what platform you are writing this for, but this may be a platform behavioral difference that needs an #ifdef.
Comment 5 Dave Hyatt 2007-11-29 12:49:27 PST
Comment on attachment 17595 [details]
patch for fixing the cursor height issue 

I would need to check to see what OS X textareas do, but generic editing (like TextEdit) uses the full height of the root line.
Comment 6 Dave Hyatt 2007-11-29 12:54:56 PST
Bring up TextEdit on the Mac and set spacing to "double."  You will see that our behavior matches TextEdit.  If we decide this behavior needs to be different on non-Mac platforms, there's a lot more work to do than merely patching caretRect, since selection also behaves this way.
Comment 7 Dave Hyatt 2007-11-29 13:09:05 PST
If you're going for more Windows-like behavior, I would suggest that RenderTheme have a new method like "usesRaggedSelection."  Then many places will have to check that and change their behavior accordingly.  You would probably want to turn off selection gap filling off as well, as it no longer makes much sense if your selection is going to be ragged and ugly (ducks).


Comment 8 amit 2007-11-29 17:38:16 PST
I was only testing on windows platform, but I guess I did miss the selection issue. 
Earlier comment on this issue suggested that the behavior on IE is better in this respect and I guess thats what I tried to achieve (except for the selection).
I'll see if I can fix that as well.
Comment 9 victor.singh1983 2012-01-11 23:35:52 PST
Hi 

Applied this patch on Icecream sandwich webkit.


But if we change the font size then cursor size does,nt change

We have tried this use case on midas demo

http://www-archive.mozilla.org/editor/midasdemo/

Please help this new bie


Thanks and Regards
Victor
Comment 10 victor.singh1983 2012-01-12 01:07:27 PST
(In reply to comment #9)
> Hi 
> 
> Applied this patch on Icecream sandwich webkit.
> 
> 
> But if we change the font size then cursor size does,nt change
> 
> We have tried this use case on midas demo
> 
> http://www-archive.mozilla.org/editor/midasdemo/
> 
> Please help this new bie
> 
> 
> Thanks and Regards
> Victor

While debugging i have found that

locclcaretRect() in RenderTextControlInnerBlock never gets called
Comment 11 victor.singh1983 2012-01-12 01:22:01 PST
(In reply to comment #9)
> Hi 
> 
> Applied this patch on Icecream sandwich webkit.
> 
> 
> But if we change the font size then cursor size does,nt change
> 
> We have tried this use case on midas demo
> 
> http://www-archive.mozilla.org/editor/midasdemo/
> 
> Please help this new bie
> 
> 
> Thanks and Regards
> Victor

While debugging i have found that

locclcaretRect() in RenderTextControlInnerBlock never gets called
Comment 12 victor.singh1983 2012-01-13 05:43:15 PST
Hi,

I have made change that is make a new function to handle fontFace and fontSize

and checked from editor if selected text is zero if not then insertHTML

 "<font size=\""+ fontSizeValue + "\" face=\"" + fontNameValue + "\">" + "&#27;</font>";

However it creates one issue that if user keeps changing the fonts at the same cursor position then we will have lot of font elements

so need to remove such font element which have only escape chracter inside them

Thanks Victor
Comment 13 Radar WebKit Bug Importer 2016-12-01 12:37:27 PST
<rdar://problem/29464647>
Comment 14 Ahmad Saleem 2022-07-12 07:29:12 PDT
I am not able to reproduce this bug in Safari 15.5 on macOS 12.4 based not attached test case and I test via following:

1) Caret Indicator is of normal size and I typed multiple words and there was no changes in the caret indicator size.
2) I hovered the cursor on the <textarea> and there was no change in cursor size. (With and without text present in the <textarea>)

Both of above tests were performed while loading the test case in Private Window. It matches with other browsers (Chrome Canary 105 and Firefox Nightly 104).

I think it got fixed along the way (if I am testing it correctly) and can be marked as "RESOLVED CONFIGURATION CHANGED". If I am testing incorrectly, please retest accordingly or add some more instruction clarity. Thanks!
Comment 15 Alexey Proskuryakov 2022-07-12 20:29:28 PDT
This is still a problem. Just hit Return in the attached test case, and the cursor height changes to line-height. Chrome works correctly.
Comment 16 Ahmad Saleem 2023-06-03 17:18:36 PDT
(In reply to Alexey Proskuryakov from comment #15)
> This is still a problem. Just hit Return in the attached test case, and the
> cursor height changes to line-height. Chrome works correctly.

@ap - I am not able to reproduce this on 'Return' as well in Safari 16.5.
Comment 17 Alexey Proskuryakov 2023-06-03 17:59:38 PDT
Still reproduces for me. The cursor becomes much bigger after pressing Return.
Comment 18 Joone Hur 2023-11-15 18:08:51 PST

*** This bug has been marked as a duplicate of bug 249621 ***