Bug 50049

Summary: [Qt] Add SelectAll option to the context menu for the editor
Product: WebKit Reporter: Yi Shen <max.hong.shen>
Component: WebKit QtAssignee: Yi Shen <max.hong.shen>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, benjamin, commit-queue, eric, kling, webkit-ews, webkit.review.bot
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 22946    
Bug Blocks:    
Attachments:
Description Flags
proposal fix
none
fix webkit2 build
kenneth: review+, commit-queue: commit-queue-
fix the commit bot issue casued by folder name change none

Description Yi Shen 2010-11-24 20:25:52 PST
When context menu pops up on the editor like textarea, user can use the selectall option to select all the text in the editor
Comment 1 Yi Shen 2010-11-24 20:32:23 PST
Created attachment 74828 [details]
proposal fix
Comment 2 Early Warning System Bot 2010-11-24 20:50:46 PST
Attachment 74828 [details] did not build on qt:
Build output: http://queues.webkit.org/results/6318039
Comment 3 Yi Shen 2010-11-25 10:23:46 PST
Created attachment 74890 [details]
fix webkit2 build
Comment 4 Eric Seidel (no email) 2010-12-09 23:38:28 PST
Is this a standard Qt context menu option?
Comment 5 Yi Shen 2010-12-10 06:36:24 PST
(In reply to comment #4)
> Is this a standard Qt context menu option?
I think so. I tried some applications in the qtdemo on linux, and I can see the select all option in the context menu for all the editable component.
Comment 6 Kenneth Rohde Christiansen 2010-12-24 02:26:59 PST
Comment on attachment 74890 [details]
fix webkit2 build

View in context: https://bugs.webkit.org/attachment.cgi?id=74890&action=review

mac also seems to have this for TextArea and input fields, so r=me

> WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:302
> +    notImplemented();
> +    return "Select All";

Why does this have notImplemented???
Comment 7 WebKit Commit Bot 2010-12-24 02:40:50 PST
Comment on attachment 74890 [details]
fix webkit2 build

Rejecting attachment 74890 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--no-update', '--non-interactive', 74890]" exit_code: 2
Last 500 characters of output:
-------------------------
|Index: WebKitTools/QtTestBrowser/mainwindow.cpp
|===================================================================
|--- WebKitTools/QtTestBrowser/mainwindow.cpp	(revision 72736)
|+++ WebKitTools/QtTestBrowser/mainwindow.cpp	(working copy)
--------------------------
No file to patch.  Skipping patch.
1 out of 1 hunk ignored

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/7272148
Comment 8 Eric Seidel (no email) 2010-12-24 08:34:19 PST
Sorry, WebKitTools recently got renamed to Tools.  There has been some discussion of modifying svn-apply to be able to fix patches during apply, but until that happens we need to fix this patch manually to apply to Tools instead of WebKitTools.
Comment 9 Yi Shen 2010-12-30 09:01:53 PST
(In reply to comment #8)
> Sorry, WebKitTools recently got renamed to Tools.  There has been some discussion of modifying svn-apply to be able to fix patches during apply, but until that happens we need to fix this patch manually to apply to Tools instead of WebKitTools.

Eric, thanks for your explanation :)
Comment 10 Yi Shen 2011-01-03 12:10:02 PST
Created attachment 77835 [details]
fix the commit bot issue casued by folder name change
Comment 11 WebKit Commit Bot 2011-01-03 15:42:45 PST
Comment on attachment 77835 [details]
fix the commit bot issue casued by folder name change

Clearing flags on attachment: 77835

Committed r74941: <http://trac.webkit.org/changeset/74941>
Comment 12 WebKit Commit Bot 2011-01-03 15:42:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Review Bot 2011-01-03 16:18:42 PST
http://trac.webkit.org/changeset/74941 might have broken Windows Release (Build) and Windows Debug (Build)
Comment 14 Benjamin Poulain 2011-01-05 05:41:32 PST
I have mixed feeling about this patch. I am affraid that will cause existing apps to have the item "select all" two times in their menu.

Yi, would you mind checking the code of Konqueror, Rekonq and Arora regarding this?
Comment 15 Yi Shen 2011-01-07 08:38:25 PST
(In reply to comment #14)
> I have mixed feeling about this patch. I am affraid that will cause existing apps to have the item "select all" two times in their menu.
> 
> Yi, would you mind checking the code of Konqueror, Rekonq and Arora regarding this?

Hi Benjamin Poulain, I will check it and let you know.
Comment 16 Yi Shen 2011-01-13 07:45:56 PST
(In reply to comment #14)
> I have mixed feeling about this patch. I am affraid that will cause existing apps to have the item "select all" two times in their menu.
> 
> Yi, would you mind checking the code of Konqueror, Rekonq and Arora regarding this?

Sorry for late reply. I tested Arora and didn't see any problem. (Sorry, I don't have KDE env to run Konqueror,Rekonq). Benjamin Poulain, let me know if you think it is still necessary to test Konqueror and Rekonq as well.
Comment 17 Benjamin Poulain 2011-01-14 05:04:32 PST
(In reply to comment #16)
> Sorry for late reply. I tested Arora and didn't see any problem. (Sorry, I don't have KDE env to run Konqueror,Rekonq). Benjamin Poulain, let me know if you think it is still necessary to test Konqueror and Rekonq as well.

I mentioned the problem to the devs of Rekonq. That should be ok.

Lets hope no other big projects rely on this kind of specific behavior so the patch does not cause too many problems.