Bug 62346

Created attachment 96528 [details] Patch

Comment on attachment 96528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96528&action=review As with other WebKit2 patches, I'm mostly reviewing for style issues, as I'm not familiar with WebKit2 yet. WebEditorClientEfl.cpp doesn't use anything EFL-related (and I couldn't see any EFL-specific behaviour there, too). Is it possible to share this code with other WebKit2 ports? > Source/WebKit2/WebProcess/WebCoreSupport/efl/WebEditorClientEfl.cpp:44 > +bool WebEditorClient::executePendingEditorCommands(Frame* frame, Vector<WTF::String> pendingEditorCommands, bool allowTextInsertion) > +{ > + Vector<Editor::Command> commands; > + for (size_t i = 0; i < pendingEditorCommands.size(); i++) { I'd use Vector::iterator here (Vector::begin(), Vector::end(), etc), for both pendingEditorCommands and commands.

(In reply to comment #2) > (From update of attachment 96528 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96528&action=review > > As with other WebKit2 patches, I'm mostly reviewing for style issues, as I'm not familiar with WebKit2 yet. > > WebEditorClientEfl.cpp doesn't use anything EFL-related (and I couldn't see any EFL-specific behaviour there, too). Is it possible to share this code with other WebKit2 ports? > > > Source/WebKit2/WebProcess/WebCoreSupport/efl/WebEditorClientEfl.cpp:44 > > +bool WebEditorClient::executePendingEditorCommands(Frame* frame, Vector<WTF::String> pendingEditorCommands, bool allowTextInsertion) > > +{ > > + Vector<Editor::Command> commands; > > + for (size_t i = 0; i < pendingEditorCommands.size(); i++) { > > I'd use Vector::iterator here (Vector::begin(), Vector::end(), etc), for both pendingEditorCommands and commands. We referenced WebKit2/Gtk and I realized that this code only used for Gtk.(Anyway it looks fine for Efl.) I think that It's better to removed for initial drop and investigate more.

Created attachment 96721 [details] Patch

Comment on attachment 96721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96721&action=review > Source/WebKit2/WebProcess/WebCoreSupport/efl/WebErrorsEfl.cpp:4 > + * Portions Copyright (c) 2010 Motorola Mobility, Inc. All rights reserved. > + * Copyright (C) 2011 Igalia S.L. Did you really copy this code? or why do you have all those copyrights?

Created attachment 96726 [details] Patch

(In reply to comment #5) > (From update of attachment 96721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96721&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/efl/WebErrorsEfl.cpp:4 > > + * Portions Copyright (c) 2010 Motorola Mobility, Inc. All rights reserved. > > + * Copyright (C) 2011 Igalia S.L. > > Did you really copy this code? or why do you have all those copyrights? Thanks and I cleared it.

The commit-queue encountered the following flaky tests while processing attachment 96726 [details]: http/tests/local/stylesheet-and-script-load-order.html bug 62450 (author: koivisto@iki.fi) The commit-queue is continuing to process your patch.

Comment on attachment 96726 [details] Patch Clearing flags on attachment: 96726 Committed r88540: <http://trac.webkit.org/changeset/88540>

All reviewed patches have been landed. Closing bug.