Bug 45828 – [Qt] When the selected element is editable, the insertion of a tab character is failing.

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Ragner Magalhaes]

The test LayoutTests/fast/events/keypress-insert-tab.html is failing because
the property to enable and disable the tab key cycles through elements on the page
is not implemented in LayoutTestController class.

[Ragner Magalhaes]

Created attachment 67709 [details]
Adjust the Qt API to enable/disable the tab key cycles through elements on the page

[Antonio Gomes]

Comment on attachment 67709 [details]
Adjust the Qt API to enable/disable the tab key cycles through elements on the page

You do not have to add this API to qwebpage just to get this test working. you can either:

1) Add to qwebsettings
2) or WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt

Do you have a real test case for it?

Should it affect all pages? or just one page?

Maybe is needs further discussion?

[Diego Gonzalez]

(In reply to comment #2)
> (From update of attachment 67709 [details])
> You do not have to add this API to qwebpage just to get this test working. you can either:
> 
> 1) Add to qwebsettings

I think add a setting could be a good idea

> 2) or WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt
> 
> Do you have a real test case for it?
> 
> Should it affect all pages? or just one page?
> 
> Maybe is needs further discussion?

[Ragner Magalhaes]

(In reply to comment #2)
> (From update of attachment 67709 [details])
> You do not have to add this API to qwebpage just to get this test working. you can either:
> 
> 1) Add to qwebsettings

In the win platform it is implemented in the API(WebKit/win/WebView)
but in the gtk it is implemented as settings( WebKit/gtk/webkit/webkitwebsettings)
so a just choose to follow the win platform ...

bad choice? 

> 2) or WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt

If I choose this option I don't know how I could check the tabKeyCyclesThroughElements() property
from(WebCore/page/Page) to insert the tab character.

The proposed patch check it in 
WebKit/qt/WebCoreSupport/EditorClientQt.cpp
---
         case VK_TAB:
+            if (!m_page->tabKeyCyclesThroughElements())
+                frame->editor()->command("InsertTab").execute();
             return;
---



> 
> Do you have a real test case for it?

Just if someone wants to insert a tab character in the edit box,
it is necessary ...

> 
> Should it affect all pages? or just one page?

I didn't understand this question ... sorry 

I think it should affect focused item from current page ... :(

> 
> Maybe is needs further discussion?

[Antonio Gomes]

(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 67709 [details] [details])
> > You do not have to add this API to qwebpage just to get this test working. you can either:
> > 
> > 1) Add to qwebsettings
> 
> In the win platform it is implemented in the API(WebKit/win/WebView)
> but in the gtk it is implemented as settings( WebKit/gtk/webkit/webkitwebsettings)
> so a just choose to follow the win platform ...
> 
> bad choice? 

It looks like more appropriated to QWebSettings, imo. Kenneth, Simon?

> The proposed patch check it in 
> WebKit/qt/WebCoreSupport/EditorClientQt.cpp
> ---
>          case VK_TAB:
> +            if (!m_page->tabKeyCyclesThroughElements())
> +                frame->editor()->command("InsertTab").execute();
>              return;
> ---
> 
> > Do you have a real test case for it?
> 
> Just if someone wants to insert a tab character in the edit box,
> it is necessary ...

But TAB insertion into edit box is not related to TabCycle, is it? Could you elaborate?

> > Should it affect all pages? or just one page?
> 
> I didn't understand this question ... sorry 

In QWebSettings, you have per-page settings, and global settings:

- Per-page: you set for one QWebPage, and others will not get affected;
- Global: you set once, and all pages get affected.

[Diego Gonzalez]

Chromium implements it as a API methods. Mac uses it as a private methods. Maybe implement in the API should be considereated, but we really need more user cases.

[Ragner Magalhaes]

> > 
> > > Do you have a real test case for it?
> > 
> > Just if someone wants to insert a tab character in the edit box,
> > it is necessary ...
> 
> But TAB insertion into edit box is not related to TabCycle, is it? Could you elaborate?
> 

I think so,
if the edit box is not editable every tab key pressed will do a TabCyle,
otherwise it is necessary to check the tabKeyCyclesThroughElements
property to do the TabCycle, or just insert a tab character,
if this property is FALSE.


> > > Should it affect all pages? or just one page?
> > 
> > I didn't understand this question ... sorry 
> 
> In QWebSettings, you have per-page settings, and global settings:
> 
> - Per-page: you set for one QWebPage, and others will not get affected;
> - Global: you set once, and all pages get affected.

I think, should be a global settings, but it is just my opinion,
and I am just a newbie here ... ;)

[Kenneth Rohde Christiansen]

I'm not sure about this. What if you want to add a "next" button to a virtual keyboard to do the same. How would I go around doing that?

[Kenneth Rohde Christiansen]

Comment on attachment 67709 [details]
Adjust the Qt API to enable/disable the tab key cycles through elements on the page

r-, this should not be part of the QWebPage... QWebSetting makes more sense.

[Antonio Gomes]

Comment on attachment 67709 [details]
Adjust the Qt API to enable/disable the tab key cycles through elements on the page

View in context: https://bugs.webkit.org/attachment.cgi?id=67709&action=review

Some comments below:

- If we have no real world use case for this API, I would suggesting not adding to QWebPage or QWebSettings, but DumpRenderTreeSupportQt.cpp|h instead. So here you would do:

> WebKit/qt/Api/qwebpage.cpp:2932
> +    \property QWebPage::tabKeyCyclesThroughElements
> +    \brief whether the tab key cycles through elements on the page.
> +
> +    If enabled, pressing the tab key will focus the next focusable web element;
> +    otherwise the web page will interpret tab key as normal key presses.
> +    If the selected element is editable, the tab key will cause the insertion of
> +    a tab character.

looks like a qwebsettings thing. Also please make it explicit what the default value is.

> WebKit/qt/WebCoreSupport/EditorClientQt.cpp:424
> +            if (!m_page->tabKeyCyclesThroughElements())
> +                frame->editor()->command("InsertTab").execute();

Note, this code was refactored by bug 51306.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.