This is the reason that fast/events/keypress-insert-tab.html is skipped.
Created attachment 41363 [details] Patch for this issue I've attached a patch exposing this method in the GTK+ API. I marked the introduction version as 1.1.16, because I was unsure of release plans.
Comment on attachment 41363 [details] Patch for this issue > 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.
Created attachment 41381 [details] Page for this issue (WebKitWebSettings version) I've attached another patch with this setting in WebKitWebSettings. I initially wavered as to where to put this, but chose WebKitWebView. My reasoning was that this would be the first property of WebKitWebSettings that did not have a direct mapping to WebCore::Settings. I'll leave both patches here so they can duke it out. Also, in my previous patch, I wasn't aiming to fix the formatting. The length of the new method name was longer than the others, so to keep everything lined up I had to move the rest over.
Comment on attachment 41381 [details] Page for this issue (WebKitWebSettings version) > + * > + * Whether the tab key cycles through elements on the page. > + * > + * If @flag is %TRUE, pressing the tab key will focus the next element in > + * the @web_view. If @flag is %FALSE, the @web_view will interpret tab > + * key presses as normal key presses. If the selected element is editable, the What's the difference between pressing the tab key and the tab key being interpreted as a normal key press? > + "tab-key-cycles-through-elements", &tabKeyCyclesThroughElements, I think we prefix boolean settings with enable-. +1 for me. Xan or Gustavo may want to look into this as well.
Comment on attachment 41381 [details] Page for this issue (WebKitWebSettings version) (In reply to comment #4) > (From update of attachment 41381 [details]) > > + * If @flag is %TRUE, pressing the tab key will focus the next element in > > + * the @web_view. If @flag is %FALSE, the @web_view will interpret tab > > + * key presses as normal key presses. If the selected element is editable, the > > What's the difference between pressing the tab key and the tab key being > interpreted as a normal key press? As I understand it, TRUE will cause tab presses while on a textarea to move the cursor on to the next element, while FALSE will add \t to the text. > > + "tab-key-cycles-through-elements", &tabKeyCyclesThroughElements, > > I think we prefix boolean settings with enable-. I think in this case, the setting is more akin to the likes of 'auto-load-images', as in, this is no feature that may be enable/disabled, but decision on behavior of an already enabled feature (interpreting tab key presses). I'm in favor of keeping the name as is in the patch, then. I'm going to go ahead and say r=me.
Comment on attachment 41381 [details] Page for this issue (WebKitWebSettings version) Rejecting patch 41381 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Gustavo Noronha', '--force']" exit_code: 1 Last 500 characters of output: f 5 hunks FAILED -- saving rejects to file WebKit/gtk/webkit/webkitwebsettings.cpp.rej patching file WebKit/gtk/webkit/webkitwebview.cpp Hunk #1 FAILED at 2392. Hunk #2 FAILED at 2422. Hunk #3 succeeded at 2451 with fuzz 2 (offset -2 lines). Hunk #4 FAILED at 2544. 3 out of 4 hunks FAILED -- saving rejects to file WebKit/gtk/webkit/webkitwebview.cpp.rej patching file WebKitTools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebKitTools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp
The patch is simply out of date. Someone could post a new patch which works with tip of tree, or someone could manually apply the patch and land this by hand. I recommend simply posting an up-to-date patch and getting a new r+ and cq+
Created attachment 42723 [details] Patch for this issue (against ToT) I've attached a patch against tip of trunk.
Comment on attachment 42723 [details] Patch for this issue (against ToT) Clearing flags on attachment: 42723 Committed r50642: <http://trac.webkit.org/changeset/50642>
All reviewed patches have been landed. Closing bug.