In http://crbug.com/new, attempt to drag some text from a TEXTAREA to an empty text input. It will fail, and will raise an assert in debug builds: ASSERT(oldLength >= selectionLength); I think this is because WebCore is calculating oldLength from the wrong element; it looks like it's calculating the length of the text field being dragged to rather than the text field being dragged from. However, I'm not familiar enough with this code to say for sure.
Created attachment 54411 [details] Patch
Comment on attachment 54411 [details] Patch In what way is this "incorrect arithmetic"? Can we re-title the bug to be clearer? I don't understand the comment "No need to calculate the selection length in a case of selection drag-and-drop." - The comment doesn't say why focused() is a reliable way to detect the fact that selected text is being dragged within the element. - The comment doesn't say why setting selectionLength to 0 is a good way to bypass the selection check entirely. The main purpose of comments is to explain why the code exists. The code can speak for itself on *what* it does. I am not convinced that setting selectionLength to 0 is the best way to handle this.
Created attachment 54498 [details] Patch (rev.2)
This seems to be a regression from Safari 4.0.5. Any idea what caused it? I am asking because whatever it was may have had other side effects that your patch will not address.
Thank you for reviewing (In reply to comment #2) > I don't understand the comment "No need to calculate the selection length in a > case of selection drag-and-drop." > - The comment doesn't say why focused() is a reliable way to detect the fact > that selected text is being dragged within the element. > - The comment doesn't say why setting selectionLength to 0 is a good way to > bypass the selection check entirely. > The main purpose of comments is to explain why the code exists. The code can > speak for itself on *what* it does. > > I am not convinced that setting selectionLength to 0 is the best way to handle > this. I updated the comments. I think the new comments explain them. And I removed the existing comment "// selection() may be a pre-edit text.". I added it when I fixed an issue for IME, but the comment is not helpful for now though it was helpful during the IME change.
Caused by <http://trac.webkit.org/changeset/48698>.
<rdar://problem/7915903>
(In reply to comment #6) > Caused by <http://trac.webkit.org/changeset/48698>. Or r48011. The assertion has been there for a long time. The code have never considered text drag-and-drop, but it worked accidentally. It seems my change exposed the problem.
Landed as r58502.
http://trac.webkit.org/changeset/58502 might have broken GTK Linux 32-bit Release
(In reply to comment #10) > http://trac.webkit.org/changeset/58502 might have broken GTK Linux 32-bit > Release I'll add the test to the skipped list for GTK and Qt.