This functionality is necesarry to pass many layout tests and eventually it would be a very useful part of the API.
Created attachment 73198 [details] Add editing dumps
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.
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.
(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.
Created attachment 74484 [details] Patch with addressing Xan's comments
(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.
(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.
Created attachment 74496 [details] Patch using g_signal_accumulator_first_wins and fixing a couple other minor issues
Comment on attachment 74496 [details] Patch using g_signal_accumulator_first_wins and fixing a couple other minor issues Yay!
Committed r72675: <http://trac.webkit.org/changeset/72675>
http://trac.webkit.org/changeset/72675 might have broken GTK Linux 64-bit Debug