WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
50737
Implement "Use Selection for Find" in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=50737
Summary
Implement "Use Selection for Find" in WebKit2
Maciej Stachowiak
Reported
2010-12-09 00:38:16 PST
Implement "Use Selection for Find" in WebKit2
Attachments
Patch
(6.10 KB, patch)
2010-12-09 00:48 PST
,
Maciej Stachowiak
no flags
Details
Formatted Diff
Diff
Patch
(6.64 KB, patch)
2010-12-09 01:40 PST
,
Maciej Stachowiak
no flags
Details
Formatted Diff
Diff
Patch
(6.66 KB, patch)
2010-12-09 02:12 PST
,
Maciej Stachowiak
mitz: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1
2010-12-09 00:48:01 PST
Created
attachment 76022
[details]
Patch
Sam Weinig
Comment 2
2010-12-09 00:52:53 PST
Comment on
attachment 76022
[details]
Patch Does it makes sense to move WebKit1 over to using this as well?
Maciej Stachowiak
Comment 3
2010-12-09 01:27:23 PST
(In reply to
comment #2
)
> Does it makes sense to move WebKit1 over to using this as well?
I thought about it, but I'm not sure that would simplify the WebKit Classic implementation overall. It doesn't benefit much from being able to reuse editor command machinery, which in the WebKit2 case handles the magic of asynchronous menu validation.
Maciej Stachowiak
Comment 4
2010-12-09 01:40:26 PST
Created
attachment 76024
[details]
Patch
mitz
Comment 5
2010-12-09 01:46:00 PST
Comment on
attachment 76024
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=76024&action=review
r=me if you change enabledCopy to enabledTake…
> WebCore/editing/EditorCommand.cpp:1510 > + { "TakeFindStringFromSelection", { executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledCopy, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
I think you meant to use enabledTakeFindFromSelection here, not enabledCopy.
> WebCore/editing/mac/EditorMac.mm:200 > + systemBeep();
Not sure it’s better to call the cross-platform function here, since this is Mac code, and you can just NSBeep() without incurring the #import cost.
> WebCore/editing/mac/EditorMac.mm:204 > + NSString* nsSelectedText = m_frame->displayStringModifiedByEncoding(selectedText());
Misplaced star.
Maciej Stachowiak
Comment 6
2010-12-09 02:12:09 PST
Created
attachment 76026
[details]
Patch
Eric Seidel (no email)
Comment 7
2010-12-09 02:34:09 PST
Attachment 76024
[details]
did not build on mac: Build output:
http://queues.webkit.org/results/6955005
mitz
Comment 8
2010-12-09 07:08:31 PST
Comment on
attachment 76026
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=76026&action=review
> WebCore/editing/mac/EditorMac.mm:204 > + NSString* nsSelectedText = m_frame->displayStringModifiedByEncoding(selectedText());
Should be NSString *nsSelectedText
Maciej Stachowiak
Comment 9
2010-12-09 10:08:14 PST
Committed
r73621
: <
http://trac.webkit.org/changeset/73621
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug