Bug 116424 - Caret height is wrong for editable contents
Summary: Caret height is wrong for editable contents
Status: RESOLVED DUPLICATE of bug 249621
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: KyungTae Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-19 19:39 PDT by KyungTae Kim
Modified: 2023-11-15 18:13 PST (History)
9 users (show)

See Also:


Attachments
test case (159 bytes, text/html)
2013-05-19 20:42 PDT, KyungTae Kim
no flags Details
Patch (1.44 KB, patch)
2013-05-19 20:44 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (591.85 KB, application/zip)
2013-05-20 01:12 PDT, Build Bot
no flags Details
Patch (1.52 KB, patch)
2013-05-21 22:23 PDT, KyungTae Kim
rniwa: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description KyungTae Kim 2013-05-19 19:39:27 PDT
Currently, caret's height is current line's height, 
but it looks weird for editable contents with an image and text on the same line (Or text with variable font-sizes).

For example, from the attached editable.htm, you can see the caret height is too big while editing the text.

When I checked the Firefox and the MS Word, the caret height was not a line's height, but a current object's height,
and it looks more reasonable.
Comment 1 KyungTae Kim 2013-05-19 20:42:14 PDT
Created attachment 202251 [details]
test case
Comment 2 KyungTae Kim 2013-05-19 20:44:44 PDT
Created attachment 202252 [details]
Patch
Comment 3 Build Bot 2013-05-20 01:12:02 PDT
Comment on attachment 202252 [details]
Patch

Attachment 202252 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/447106

New failing tests:
editing/selection/caret-alignment-for-vertical-text.html
Comment 4 Build Bot 2013-05-20 01:12:03 PDT
Created attachment 202263 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 5 Darin Adler 2013-05-20 10:51:53 PDT
Comment on attachment 202252 [details]
Patch

I am not sure this is right for Mac user interface, even though it might be OK on other platforms.
Comment 6 Ryosuke Niwa 2013-05-20 11:26:38 PDT
Comment on attachment 202252 [details]
Patch

Definitely not right for Mac.
Comment 7 KyungTae Kim 2013-05-20 20:39:07 PDT
(In reply to comment #6)
> (From update of attachment 202252 [details])
> Definitely not right for Mac.

Do you mean current operation is right for Mac port?

On attached test case, caret height became 1171px, 
and when window height is smaller than that,
you can't see the text while editing because the contents is scrolled up to the top - it's same on the Safari.
Is it OK for Mac port?
Comment 8 Darin Adler 2013-05-21 08:45:33 PDT
(In reply to comment #7)
> Do you mean current operation is right for Mac port?

Caret being as tall as the tallest image on a line is the correct behavior for the Mac port. You can see it in OS X applications such as TexTEdit.

> On attached test case, caret height became 1171px, 
> and when window height is smaller than that,
> you can't see the text while editing because the contents is scrolled up to the top - it's same on the Safari.
> Is it OK for Mac port?

There may be a problem there that needs to be solved, but it is not right to solve it by making the caret ignore the heights of images that are on the same lines as text. The user interface design for Mac means that the caret is supposed to be based on the line height, which can be affected by images as well as text on that line.

We’ll have to think of some other solution. Possibly, we can fix the issue that you can’t see the text when editing by changing the rules for scrolling. And we might invent some different UI for when the images are so tall that the caret as tall as the tallest image on a line is silly or absurd.

But just changing the rule to “caret heights consider only text ignoring images” is wrong for Mac UI.
Comment 9 KyungTae Kim 2013-05-21 22:23:14 PDT
Created attachment 202497 [details]
Patch
Comment 10 Ryosuke Niwa 2013-05-21 22:38:01 PDT
Did you run all layout tests?  I'll be surprised if this change doesn't cause any test failure.
Comment 11 Ryosuke Niwa 2013-08-01 19:35:21 PDT
Comment on attachment 202497 [details]
Patch

This patch needs to rebaseline all the editing tests' pixel results. We should probably make this dependent on the editing behavior instead.
Comment 12 Joone Hur 2023-11-15 18:13:14 PST

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