Bug 16118

Summary: Selection should be revealed when set programmatically
Product: WebKit Reporter: Frederico Caldeira Knabben <fredck@fredck.com>
Component: HTML EditingAssignee: Nobody <webkit-unassigned@lists.webkit.org>
Status: NEW    
Severity: Normal CC: ap@webkit.org, darin@apple.com, enrica@apple.com, justin.garcia@apple.com, leviw@chromium.org, michael.vm@gmail.com, pravind.2k4@gmail.com, rniwa@webkit.org, styu007@gmail.com, sullivan@chromium.org
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.fredck.com/bugs/safari/1462_2_reduced.html
Bug Depends on:    
Bug Blocks: 6627, 9915    

Description From 2007-11-24 10:53:17 PST
While editing a document, the current selection (or blinking caret position) should always be visible when set by code. This is the normal behavior on all kinds of editors.

Today, only IE ensures that the blinking caret is always visible. Other browsers, including Safari, allows the caret to be moved to a hidden place.

The above URL shows the problem (not compatible with IE thought).
------- Comment #1 From 2007-11-24 10:54:16 PST -------
This is a critical but for FCKeditor. For reference, here is the ticket at our side:

http://dev.fckeditor.net/ticket/1462
------- Comment #2 From 2007-11-24 12:12:30 PST -------
This is actually better demonstrated by:
http://www.fckeditor.net/_temp/browsers_bugs/safari/1462.html
------- Comment #3 From 2007-11-24 15:21:46 PST -------
(In reply to comment #2)
> This is actually better demonstrated by:
> http://www.fckeditor.net/_temp/browsers_bugs/safari/1462.html

Actually Eric, the above URL shows the problem we have today, which made us opening this ticket, but not the problem described in this ticket.

In FCKeditor, we have a custom implementation for the enter key. So by code, we generate the paragraphs and put the selection (caret) there. To make the selection visible, we are currently using scrollIntoView(false).

But, as described at bug 16117, scrollIntoView(false) has an undesired behavior in Safari, making FCKeditor unusable.

As discusses at #webkit, bug 16117 is invalid and is not supposed to be fixed, as the current is the behavior in IE. So, the alternative for us would be stop using scrollIntoView.

To do that WebKit itself must guarantee that the selection will be visible automatically, as does IE, so we came out with this bug report.

I really hope all this thing make sense. Thanks for your comments.
------- Comment #4 From 2011-05-24 22:39:43 PST -------
Is this bug about auto-scrolling to where the caret is?  That sounds like a reasonable feature to implement.
------- Comment #5 From 2011-05-25 03:49:20 PST -------
(In reply to comment #4)
> Is this bug about auto-scrolling to where the caret is?  That sounds like a reasonable feature to implement.

Yes, it's exactly that.

Basically, there should be an auto-scroll whenever a selection change takes place. The full contents of the selection must be visible (for multi-line or object selections), added by some padding space to not have the selection touching the viewport border.

Note that this should also work for nested viewports (e.g. iframe inside iframe inside window)... all of them must scroll to show the selection.
------- Comment #6 From 2011-05-25 08:52:45 PST -------
(In reply to comment #5)
> Basically, there should be an auto-scroll whenever a selection change takes place. The full contents of the selection must be visible (for multi-line or object selections), added by some padding space to not have the selection touching the viewport border.

I'm not sure about adding padding part but I see that auto-scrolling to show the selection is a reasonable behavior.  But is this really limited to design mode?  Shouldn't we auto-scroll to where selection is when selection is set programmatically in non-design mode as well?

> Note that this should also work for nested viewports (e.g. iframe inside iframe inside window)... all of them must scroll to show the selection.

Sure.

One question.  What should happen if the selected region was larger than the frame's viewport?  i.e. you select the entire document in iframe which has a vertical scroll bar.  Should we auto-scroll to where the selection extent is?
------- Comment #7 From 2011-05-25 14:15:40 PST -------
I suggest separating discussion of revealing caret and revealing non-caret selection. We do the former in most cases (like when pasting or using arrow keys), so it's an easier question to answer if we need that for programmatically moved caret.

Revealing any programmatically changed selection is a bit more questionable. For example, what if a script performs search and replace, changing the selection each time? Will we want to waste time displaying every state? I'm sure there are more cases like this.

At the same time, I am not aware of any reasons for limiting the discussion to designMode="on".
------- Comment #8 From 2011-06-08 11:52:34 PST -------
(In reply to comment #6)
> But is this really limited to design mode?  Shouldn't we auto-scroll to where selection is when selection is set programmatically in non-design mode as well?

Definitely yes. I mean... design mode should not make any difference.

> One question.  What should happen if the selected region was larger than the frame's viewport?  i.e. you select the entire document in iframe which has a vertical scroll bar.  Should we auto-scroll to where the selection extent is?

The easy answer: simply reflect the current behavior of a textarea as it works exactly as expected.
------- Comment #9 From 2011-06-08 11:53:44 PST -------
(In reply to comment #7)
> Revealing any programmatically changed selection is a bit more questionable. For example, what if a script performs search and replace, changing the selection each time? Will we want to waste time displaying every state?

If "selection" is used, then scrolling is definitely desired in this case. A "non scrollable" replace would not use selection but simply work with ranges.
------- Comment #10 From 2011-06-27 17:43:49 PST -------
Changing the title to better represent the bug.

This could be fixed with essentially a 1 line change at http://trac.webkit.org/browser/trunk/Source/WebCore/editing/FrameSelection.cpp#L207

Making this change affected 6 layout tests, 5 in fast/forms and fast/dom/34176.html. All changed in the way one would expect, by scrolling the viewport or a scrollable area to reveal the selection. The change to make the selection only be revealed when triggered by user behavior was introduced without comment or test in http://trac.webkit.org/changeset/16386. Is there any objection to making this change?
------- Comment #11 From 2011-06-27 17:46:20 PST -------
(In reply to comment #10)
> Making this change affected 6 layout tests, 5 in fast/forms and fast/dom/34176.html. All changed in the way one would expect, by scrolling the viewport or a scrollable area to reveal the selection. The change to make the selection only be revealed when triggered by user behavior was introduced without comment or test in http://trac.webkit.org/changeset/16386. Is there any objection to making this change?

There could be some performance regression if we always revealed selection.  Selection may also flick, etc...
------- Comment #12 From 2011-06-27 17:46:51 PST -------
Can we revealSelection only if it was triggered by DOM?