WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
49143
[GTK] The WebKitWebView should expose a set of signals encapsulating the behavior of the EditorClient
https://bugs.webkit.org/show_bug.cgi?id=49143
Summary
[GTK] The WebKitWebView should expose a set of signals encapsulating the beha...
Martin Robinson
Reported
2010-11-07 08:17:07 PST
This functionality is necesarry to pass many layout tests and eventually it would be a very useful part of the API.
Attachments
Add editing dumps
(895.70 KB, patch)
2010-11-07 11:56 PST
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Patch with addressing Xan's comments
(895.45 KB, patch)
2010-11-20 10:40 PST
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Patch using g_signal_accumulator_first_wins and fixing a couple other minor issues
(897.90 KB, patch)
2010-11-20 22:57 PST
,
Martin Robinson
xan.lopez
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Martin Robinson
Comment 1
2010-11-07 11:56:46 PST
Created
attachment 73198
[details]
Add editing dumps
Xan Lopez
Comment 2
2010-11-19 18:57:49 PST
Comment on
attachment 73198
[details]
Add editing dumps View in context:
https://bugs.webkit.org/attachment.cgi?id=73198&action=review
Holy huge patch Batman.
> WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:367 > {
Since you are not actually implementing this (why?) you should not add the name to the parameter.
> WebKit/gtk/webkit/webkitwebview.cpp:153 > CONSOLE_MESSAGE,
This is an ABI break I believe, don't move the signal enum aronud.
> WebKit/gtk/webkit/webkitwebview.cpp:1392 > +
Is this something like "the first handler wins" or?
> WebKit/gtk/webkit/webkitwebview.cpp:2700 >
Hrm, is it really useful for subclasses to be able to override this?
> WebKit/gtk/webkit/webkitwebview.h:154 >
This would be an ABI break; you have to remove one padding.
Xan Lopez
Comment 3
2010-11-19 19:18:26 PST
Comment on
attachment 73198
[details]
Add editing dumps View in context:
https://bugs.webkit.org/attachment.cgi?id=73198&action=review
>> WebKit/gtk/webkit/webkitwebview.cpp:1392 >> + > > Is this something like "the first handler wins" or?
I think this is indeed a "first wins" type of deal. Why is it needed here? In any case, a built-in accumulator like this was added to glib recently.
>> WebKit/gtk/webkit/webkitwebview.cpp:2700 >> > > Hrm, is it really useful for subclasses to be able to override this?
Eh, nevermind, I guess you need at least one handler.
Martin Robinson
Comment 4
2010-11-20 09:24:21 PST
(In reply to
comment #3
)
> > Is this something like "the first handler wins" or? > I think this is indeed a "first wins" type of deal. Why is it needed here? In any case, a built-in accumulator like this was added to glib recently.
Thanks for the review! The only built in handler I see in GLib 2.26 is g_signal_accumulator_true_handled, which is a little different. That one is not first-handler-wins, but first handler returning TRUE stops the emission. I'm addressing your other comments now.
Martin Robinson
Comment 5
2010-11-20 10:40:12 PST
Created
attachment 74484
[details]
Patch with addressing Xan's comments
Xan Lopez
Comment 6
2010-11-20 14:52:39 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > > Is this something like "the first handler wins" or? > > I think this is indeed a "first wins" type of deal. Why is it needed here? In any case, a built-in accumulator like this was added to glib recently. > > Thanks for the review! The only built in handler I see in GLib 2.26 is g_signal_accumulator_true_handled, which is a little different. That one is not first-handler-wins, but first handler returning TRUE stops the emission. I'm addressing your other comments now.
I didn't meant that one, there's g_signal_accumulator_first_wins, which returns whatever the first accumulator returns.
Xan Lopez
Comment 7
2010-11-20 15:00:19 PST
(In reply to
comment #6
)
> I didn't meant that one, there's g_signal_accumulator_first_wins, which returns whatever the first accumulator returns.
I meant the first handler, of course.
Martin Robinson
Comment 8
2010-11-20 22:57:18 PST
Created
attachment 74496
[details]
Patch using g_signal_accumulator_first_wins and fixing a couple other minor issues
Xan Lopez
Comment 9
2010-11-23 15:44:11 PST
Comment on
attachment 74496
[details]
Patch using g_signal_accumulator_first_wins and fixing a couple other minor issues Yay!
Martin Robinson
Comment 10
2010-11-24 08:44:09 PST
Committed
r72675
: <
http://trac.webkit.org/changeset/72675
>
WebKit Review Bot
Comment 11
2010-11-24 09:26:20 PST
http://trac.webkit.org/changeset/72675
might have broken GTK Linux 64-bit Debug
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