Bug 22025 - range.deleteContents() does not collapse the selection
Summary: range.deleteContents() does not collapse the selection
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-31 19:20 PDT by brad
Modified: 2009-04-01 16:30 PDT (History)
3 users (show)

See Also:


Attachments
Reproducable Test Page (deleted)
2008-10-31 19:21 PDT, brad
no flags Details
New Test Case (1.61 KB, text/html)
2009-04-01 10:59 PDT, brad
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description brad 2008-10-31 19:20:18 PDT
Calling getSelection().getRangeAt().collapse(true) does nothing and getSelection().getRangeAt().collapsed is never set to true.

Also, it is impossible to unhighlight text if you set the innerHTML of a node that is selected (or cursor is in) and then put the cursor back to the normal position without using hacks. Webkit will incorrectly highlight text that should not be highlighted. You cannot unhighlight this text.

1) build some text
2) put the cursor in the text
3) insert a marker at cursor location
4) set the innerHTML to itself
5) call window.find to locate the marker
6) call range.deleteContents()
7) the next character after the marker will be highlighted. You cannot unhighlight it.
8) .collapse(true) doesnt work
9) setting the end and start of the range does not work
10) the only thing that works is calling setEndBefore and setStartBefore
Comment 1 brad 2008-10-31 19:21:38 PDT
Created attachment 24829 [details]
Reproducable Test Page
Comment 2 Eric Seidel (no email) 2009-02-06 10:19:26 PST
This sounds like a fun one. :)  I'll have to run it in the debugger.  We'll also have to re-write the test case to have fewer alerts :)

Thank you for the useful test case!
Comment 3 Eric Seidel (no email) 2009-02-06 10:24:10 PST
I suspect that the bug is in deleteContents.  I would expect it to collapse the selection after deleting it.

PassRefPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionCode& ec)
is an unnecessarily complicated function which should be cleaned up to fix this bug.
Comment 4 Eric Seidel (no email) 2009-03-16 17:11:55 PDT
	window.getSelection().getRangeAt(0).setEnd(mybody, 0);
	window.getSelection().getRangeAt(0).setStart(mybody, 0);

That code only works in FF.  Ranges returned from getRangeAt() in WebKit are not tied to the Selection object like they are in Firefox.  So you end up modifying a temporary range object, not the selection itself.

You can use setBaseAndExtent to modify the selection in WebKit.

We're currently considering redesigning the whole Selection API, since Firefox's is rather confusing (and IE's is similarly confusing, and WebKit's is a strange broken hybrid of the two)
Comment 5 Eric Seidel (no email) 2009-03-16 17:12:43 PDT
Likewise 	window.getSelection().getRangeAt(0).collapse(true); does not affect the selection, only the temporary range object returned by getRangeAt()
Comment 6 Eric Seidel (no email) 2009-03-16 17:38:34 PDT
I'm not sure that range.deleteContents() should modify the selection at all.

As you can see by this test:
<body>
<div id="test" contentEditable>a<span id="bspan">b</span>c</div>
<script>
var testDiv = document.getElementById("test");
var bspan = document.getElementById("bspan");
var sel = window.getSelection();
sel.selectAllChildren(testDiv);
bspan.parentNode.removeChild(bspan);
</script>
b was removed dynamically, but "ac" is still selected.
</body>


I don't think this is actually a bug, except in that our selection API is different from mozilla's (and thus confusing).  We need to fix our selection API, but "tied ranges" I don't think are the answer.
Comment 7 brad 2009-03-16 23:06:35 PDT
This is why I always love free source community.

It should either be changed, or a solution given, it's really all that simple.

The reason I posted this bug is no matter how hard I try I can't figure it out.
This whole pursuit of mine is simply so I can offer a solution to our clients a WYSIWYG that my company would allow, and the only ones they allow don't work in WebKit browsers because of implementations like this.

The only WYSIWYG editors are ones that don't use content editable regions and actually fire events while you type, and even then the cut/paste is still broken. (I have a post about how the copy/paste functions don't work as documented either)

All I'm asking for is a solution, sorry if I seem hasty, but I want to say my application supports WebKit but because of the restraints of what I would call "bugs", I am unable to help in the spread of this tool.

I personally use Google Chrome (prefer the UI over safari), but for things like this, I can't ask the same of my customers.

I'm just frustrated at this point, sorry for the rant. I am still under the impression that there is literally no way in WebKit to correctly highlight content for the user, and delete it from the page (which is required by most content editable WYSIWYG).
Comment 8 Julie Parent 2009-03-17 10:11:32 PDT
Are you still looking for a solution to any of these problems?  As Eric said, getRangeAt(0) provides you a temporary range to work with, if you would like the selection the user sees to become that range, you'll just to need to set it.

So, instead of:
window.getSelection().getRangeAt().collapse(true), 
use:
var sel = window.getSelection();
var range = sel.getRangeAt(0);
range.collapse(true);
sel.removeAllRanges();
sel.addRange(range);

Instead of:
window.getSelection().getRangeAt(0).setEnd(mybody, 0);
window.getSelection().getRangeAt(0).setStart(mybody, 0);
use:
window.getSelection().setBaseAndExtent(myBody, 0, myBody, 0);
Comment 9 brad 2009-04-01 10:59:27 PDT
Created attachment 29164 [details]
New Test Case
Comment 10 brad 2009-04-01 11:14:11 PDT
You marked this ticket as invalid but webkit still fails to meet expectations. If the issue is that the title says range.deleteContents() then change the title.

This is the method that all "syntax highlight as you type" programs I've seen on the net use. So I'm trying to modify them to make them work in webkit (none do).

I tried the suggestions and they don't work, because if you take out all the alerts you get javascript errors (not on the suggested code)

The issue seems to be that innerHTML = value is asynchronous and doesn't finish its job when its called. Because directly after I call .find and .deleteContents() and deleteContents() errors.

Mainly because .find('z') finds nothing.

body.innerHTML = 'z';
window.find('z'); // finds nothing
window.getSelection().getRangeAt(0).deleteContents(); // errors because no selection

I know I pretty much turned this thread into technical support but we have a demand for people to use safari and its been over a year now and no one can figure out how we can possibly support webkit (safari and now chrome)
Comment 11 Julie Parent 2009-04-01 13:03:05 PDT
I hate to suggest this, but ... setTimeout?
Comment 12 brad 2009-04-01 16:30:38 PDT
That makes me cry. The code required to make up for the instability that that introduces is so hard to manage. 

Well if you're saying thats the only solution there is for "typing for the user and removing text", then I guess that's all I get.

Maybe I can hack it to work, I don't know.