[Webkit-unassigned] [Bug 139443] [GTK] Add API to set the web view editable into WebKit2 GTK+ API
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jan 20 01:15:12 PST 2015
https://bugs.webkit.org/show_bug.cgi?id=139443
Carlos Garcia Campos <cgarcia at igalia.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #244972|review?, commit-queue? |review+, commit-queue-
Flags| |
--- Comment #18 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 244972
--> https://bugs.webkit.org/attachment.cgi?id=244972
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=244972&action=review
The implementation and the API looks good to me, we just need to improve a bit the tests.
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebViewEditor.cpp:107
> -static void testWebViewEditorCutCopyPasteNonEditable(EditorTest* test, gconstpointer)
> +static void loadTestHtml(EditorTest* test, bool contentEditable)
> {
> - static const char* selectedSpanHTML = "<html><body contentEditable=\"false\">"
> + GUniquePtr<char> selectedSpanHTML(g_strdup_printf(
> + "<html><body contentEditable=\"%s\">"
> "<span id=\"mainspan\">All work and no play <span id=\"subspan\">make Jack a dull</span> boy.</span>"
> "<script>document.getSelection().collapse();\n"
> "document.getSelection().selectAllChildren(document.getElementById('subspan'));\n"
> - "</script></body></html>";
> + "</script></body></html>", contentEditable ? "true" : "false"));
>
> + test->loadHtml(selectedSpanHTML.get(), nullptr);
> + test->waitUntilLoadFinished();
> +}
This is too generic, what I meant is that we could make a global
static const char* selectedSpanHTMLFormat = "...."
And then in every test we use the format string to get the actual HTML depedning on whether we want the content editable or not
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebViewEditor.cpp:145
>
> - test->loadHtml(selectedSpanHTML, 0);
> + g_assert(!test->isEditable());
> + test->setEditable(true);
> + g_assert(test->isEditable());
> + test->loadHtml(selectedSpanHTML, nullptr);
Can't we use the common span HTML in this case?
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebViewEditor.cpp:205
> +static void runEditorEditableCutTests(EditorTest* test, bool contentEditable)
What about something more descriptive like loadContentsAndTryToCutSelection?
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebViewEditor.cpp:229
> +static void testWebViewEditorEditableOnNonEditable(EditorTest* test, gconstpointer)
> +{
> + runEditorEditableCutTests(test, false);
> +}
> +
> +static void testWebViewEditorEditableOnContentEditable(EditorTest* test, gconstpointer)
> +{
> + runEditorEditableCutTests(test, true);
> +}
These could be the same test.
> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebViewEditor.cpp:250
> + EditorTest::add("WebKitWebView", "editable/non-editable", testWebViewEditorNonEditable);
> + EditorTest::add("WebKitWebView", "editable/editable-on-non-editable", testWebViewEditorEditableOnNonEditable);
> + EditorTest::add("WebKitWebView", "editable/editable-on-content-editable", testWebViewEditorEditableOnContentEditable);
I find the test names a bit confusing. What's the difference between non-editable and editable-on-non-editable? I guess the second one should be editable-on-non-editable-content? Maybe we can even merge the 3 tests into a single one editable/editable and do all the checks there.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150120/d0e269a6/attachment-0002.html>
More information about the webkit-unassigned
mailing list