[Webkit-unassigned] [Bug 67192] API to set initial focus in gtk webkit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67192


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #106852|review?                     |review-
               Flag|                            |




--- Comment #19 from Martin Robinson <mrobinson at webkit.org>  2011-09-10 11:39:37 PST ---
(From update of attachment 106852)
View in context: https://bugs.webkit.org/attachment.cgi?id=106852&action=review

> Source/WebKit/gtk/tests/testwebview.c:368
> +    char* uri = g_strconcat(base_uri, "bigdiv.html", NULL);

You didn't include this file...you should probably just load HTML as a string via webkit_web_view_load_string.

> Source/WebKit/gtk/tests/testwebview.c:388
> +    g_signal_connect(window, "map-event",
> +                     G_CALLBACK(map_event_cb), loop);

No need to break this line. If I'm not mistaken if you take my suggestion below, you shouldn't need to wait for the map event.

> Source/WebKit/gtk/tests/testwebview.c:412
> +    char script[] = "if (document.activeElement.id == \"link\")   \
> +                        window.scrollBy(0, 100);";

Hrm...this approach seems a little fragile. Why not just use webkit_dom_html_document_get_active_element from the DOM bindings to inspect the active element?

> Source/WebKit/gtk/tests/testwebview.c:414
> +    /* Execute the script to verify the focus */

Missing a period here.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4037
> + * Set the initial focus element for spatial(keyboard) navigation.So with spatial
> + * navigation when a page is loaded, user should be able see the initial position
> + * of focus without pressing tab/arrow key.
> + *
> + * If @forward is %TRUE, then focus is set in the forward direction i.e. focuses the first
> + * focusable element.If @forward is %FALSE, then focus is set in backward direction i.e
> + * focuses the last focusable element. 

Nice documentation. You are missing spaces after all of the periods though. I recommned replacing both "i.e." with something like this:

If @forward is %TRUE, then focus is set in the forward direction, which focuses the first focusable element.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4044
> +

Extra newline.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:4049
> +    Frame* frame = page->focusController()->focusedOrMainFrame();
> +    frame->document()->setFocusedNode(0);

No need to cache the frame here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.