[webkit-reviews] review denied: [Bug 30482] [GTK] Expose Page::tabKeyCyclesThroughElements in the API : [Attachment 41363] Patch for this issue

[bugzilla-daemon at webkit.org]

Jan Alonzo <jmalonzo at gmail.com> has denied Martin Robinson
<martin.james.robinson at gmail.com>'s request for review:
Bug 30482: [GTK] Expose Page::tabKeyCyclesThroughElements in the API
https://bugs.webkit.org/show_bug.cgi?id=30482

Attachment 41363: Patch for this issue
https://bugs.webkit.org/attachment.cgi?id=41363&action=review

------- Additional Comments from Jan Alonzo <jmalonzo at gmail.com>
> diff --git a/WebKit/gtk/webkit/webkitwebview.cpp
b/WebKit/gtk/webkit/webkitwebview.cpp
> index fb2bf35..db9fff3 100644
> --- a/WebKit/gtk/webkit/webkitwebview.cpp
> +++ b/WebKit/gtk/webkit/webkitwebview.cpp
> @@ -170,7 +170,8 @@ enum {
>      PROP_LOAD_STATUS,
>      PROP_PROGRESS,
>      PROP_ENCODING,
> -    PROP_CUSTOM_ENCODING
> +    PROP_CUSTOM_ENCODING,
> +    PROP_TAB_KEY_CYCLES_THROUGH_ELEMENTS

I think this property should be in WebSettings. Thoughts?

> +    case PROP_TAB_KEY_CYCLES_THROUGH_ELEMENTS:
> +	   g_value_set_boolean(value,
webkit_web_view_get_tab_key_cycles_through_elements(webView));
> +	   break;

Ditto with the tab key cycle changes in the WebView. 

> diff --git a/WebKit/gtk/webkit/webkitwebview.h
b/WebKit/gtk/webkit/webkitwebview.h
> index 1297695..52915f2 100644
> --- a/WebKit/gtk/webkit/webkitwebview.h
> +++ b/WebKit/gtk/webkit/webkitwebview.h
> @@ -131,241 +131,249 @@ struct _WebKitWebViewClass {
>  };
>  
>  WEBKIT_API GType
> -webkit_web_view_get_type (void);
> +webkit_web_view_get_type			       (void);
>  
>  WEBKIT_API GtkWidget *
> -webkit_web_view_new (void);
> +webkit_web_view_new				       (void);
>  
>  WEBKIT_API G_CONST_RETURN gchar *
> -webkit_web_view_get_title			   (WebKitWebView	
*web_view);
> +webkit_web_view_get_title			       (WebKitWebView	    
*web_view);
>  
>  WEBKIT_API G_CONST_RETURN gchar *
> -webkit_web_view_get_uri			   (WebKitWebView	
*web_view);
> +webkit_web_view_get_uri			       (WebKitWebView	    
*web_view);
>  

Thanks for fixing the formatting but I think they should be in a separate
patch.

>  void LayoutTestController::setTabKeyCyclesThroughElements(bool cycles)
>  {
> -    // FIXME: implement
> +    WebKitWebView* webView = webkit_web_frame_get_web_view(mainFrame);
> +    webkit_web_view_set_tab_key_cycles_through_elements(webView, cycles);

Ditto. The setting should be in WebSettings. 

r=- for now.