Created attachment 92590 [details] Test case The selectstart event fires (many times) when making a mouse selection or a keyboard selection but does not fire at all when the selection changes using "Select all" options from context/edit menus or via Ctrl/Cmd-A keyboard shortcut.
Does it fire in IE?
Yes, it fires in IE.
(In reply to comment #1) > Does it fire in IE? IE9 doesn't.
I only have Win XP at home so I can't test IE 9. It fires in IE 6 and 7. My test case isn't great: in IE you need to make sure you've got the focus on the body (eg by clicking on the text) before doing the select all.
If you need this to prevent selection, please note that WebKit supports a better way to do that (that is, -webkit-user-select CSS property).
Nothing to do with preventing selection. I do quite a bit of work with contenteditable/designMode and I'm interested in being notified whenever the selection is modified by any means: mouse, keyboard, menu option.
(In reply to comment #6) > Nothing to do with preventing selection. I do quite a bit of work with contenteditable/designMode and I'm interested in being notified whenever the selection is modified by any means: mouse, keyboard, menu option. Does selectionchange work for your use case? Since IE9 no longer fires selectstart when selecting all, I'm not sure we should change WebKit's behavior here.
(In reply to comment #4) > in IE you need to make sure you've got the focus on the body (eg by clicking on the text) before doing the select all. Ah, thanks for pointing that out. Now IE9 fires selectstart as well.
Frankly I don't care which event fires so long as one does, so selectionchange is fine by me. I assume this change is recent enough that I'll need to test against a nightly?
Now that I know IE9 also fires we should definitely fix this bug. (In reply to comment #9) > Frankly I don't care which event fires so long as one does, so selectionchange is fine by me. I assume this change is recent enough that I'll need to test against a nightly? It's shipped in Chrome 11.
Created attachment 92681 [details] fixes the bug
Created attachment 92682 [details] patch with tests
Comment on attachment 92682 [details] patch with tests View in context: https://bugs.webkit.org/attachment.cgi?id=92682&action=review > LayoutTests/fast/events/selectstart-on-selectall.html:13 > +var selectstartEvent = null; This variable is not used. > Source/WebCore/editing/FrameSelection.cpp:1403 > + selectElement->selectAll(); Need "return" after this line?
Created attachment 92732 [details] Fixed per comments
(In reply to comment #13) > (From update of attachment 92682 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92682&action=review > > > LayoutTests/fast/events/selectstart-on-selectall.html:13 > > +var selectstartEvent = null; > > This variable is not used. Removed. > > Source/WebCore/editing/FrameSelection.cpp:1403 > > + selectElement->selectAll(); > > Need "return" after this line? Oops, a good catch!
Comment on attachment 92732 [details] Fixed per comments Looks good.
(In reply to comment #16) > (From update of attachment 92732 [details]) > Looks good. Thanks for the review!
Committed r86041: <http://trac.webkit.org/changeset/86041>
http://trac.webkit.org/changeset/86041 might have broken Leopard Intel Debug (Tests) The following tests are not passing: fast/workers/storage/use-same-database-in-page-and-workers.html http/tests/appcache/fail-on-update-2.html http/tests/appcache/fail-on-update.html http/tests/misc/favicon-loads-with-icon-loading-override.html storage/domstorage/localstorage/storagetracker/storage-tracker-2-create.html storage/domstorage/localstorage/storagetracker/storage-tracker-3-delete-all.html svg/animations/animate-path-nested-transforms.html