Bug 16118 - Selection should be revealed when set programmatically
Summary: Selection should be revealed when set programmatically
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.fredck.com/bugs/safari/146...
Keywords:
Depends on:
Blocks: 6627 9915
  Show dependency treegraph
 
Reported: 2007-11-24 10:53 PST by webkit
Modified: 2012-08-06 14:00 PDT (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description webkit 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 webkit 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 Eric Seidel (no email) 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 webkit 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 Ryosuke Niwa 2011-05-24 22:39:43 PDT
Is this bug about auto-scrolling to where the caret is?  That sounds like a reasonable feature to implement.
Comment 5 webkit 2011-05-25 03:49:20 PDT
(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 Ryosuke Niwa 2011-05-25 08:52:45 PDT
(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 Alexey Proskuryakov 2011-05-25 14:15:40 PDT
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 webkit 2011-06-08 11:52:34 PDT
(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 webkit 2011-06-08 11:53:44 PDT
(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 Levi Weintraub 2011-06-27 17:43:49 PDT
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 Ryosuke Niwa 2011-06-27 17:46:20 PDT
(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 Ryosuke Niwa 2011-06-27 17:46:51 PDT
Can we revealSelection only if it was triggered by DOM?