Merge ~santoshbit2007/oxide:focus_issue into oxide:master

Proposed by Santosh on 2016-07-29
Status: Merged
Approved by: Chris Coulson on 2016-09-19
Approved revision: a8742c91e126f264e7f25b11f35906ade893537b
Merge reported by: Santosh
Merged at revision: not available
Proposed branch: ~santoshbit2007/oxide:focus_issue
Merge into: oxide:master
Diff against target: 114 lines (+89/-2)
3 files modified
qt/quick/oxide_qquick_contents_view.cc (+3/-2)
qt/tests/qmltests/core/tst_focus.html (+20/-0)
qt/tests/qmltests/core/tst_focus.qml (+66/-0)
Reviewer Review Type Date Requested Status
Chris Coulson 2016-07-29 Approve on 2016-09-19
Review via email: mp+301475@code.launchpad.net

Commit message

Decide focus state of webview based on activeFocusItem check of window
instead of activeFocus.

LP: #1599771

Description of the change

Decide focus state of webview based on activeFocusItem check of window
instead of activeFocus.

LP: #1599771

To post a comment you must log in.
Chris Coulson (chrisccoulson) wrote :

Thanks for this - there's a few issues with the test. I've left some comments inline.

review: Needs Fixing
Chris Coulson (chrisccoulson) wrote :

Thanks. This looks mostly ok, but there is one change I forgot to ask for in the initial review - could you please move the test to qt/tests/qmltests/web_platform?

Once that's done, feel free to push this.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/qt/quick/oxide_qquick_contents_view.cc b/qt/quick/oxide_qquick_contents_view.cc
2index afa7525..cd4ba2c 100644
3--- a/qt/quick/oxide_qquick_contents_view.cc
4+++ b/qt/quick/oxide_qquick_contents_view.cc
5@@ -104,8 +104,9 @@ bool ContentsView::IsVisible() const {
6 }
7
8 bool ContentsView::HasFocus() const {
9- return item_->hasActiveFocus() &&
10- (item_->window() ? item_->window()->isActive() : false);
11+ QQuickWindow* window = item_->window();
12+ return window ? (window->isActive() &&
13+ (window->activeFocusItem() == item_)) : false;
14 }
15
16 QRect ContentsView::GetBounds() const {
17diff --git a/qt/tests/qmltests/core/tst_focus.html b/qt/tests/qmltests/core/tst_focus.html
18new file mode 100644
19index 0000000..8e54900
20--- /dev/null
21+++ b/qt/tests/qmltests/core/tst_focus.html
22@@ -0,0 +1,20 @@
23+<html>
24+<body>
25+<center>
26+ Enter your name: <input type="text" id="input" onfocusout="focusOff()"
27+ onfocusin="focusIn()" autofocus>
28+</center>
29+ <script>
30+ function focusOff() {
31+ var e = new CustomEvent("oxidefocusstate", { bubbles: true, detail: { status: false } });
32+ document.documentElement.dispatchEvent(e);
33+ }
34+
35+ function focusIn() {
36+ var e = new CustomEvent("oxidefocusstate", { bubbles: true, detail: { status: true } });
37+ document.documentElement.dispatchEvent(e);
38+ }
39+ </script>
40+</body>
41+</html>
42+
43diff --git a/qt/tests/qmltests/core/tst_focus.qml b/qt/tests/qmltests/core/tst_focus.qml
44new file mode 100644
45index 0000000..d4b221e
46--- /dev/null
47+++ b/qt/tests/qmltests/core/tst_focus.qml
48@@ -0,0 +1,66 @@
49+import QtQuick 2.0
50+import QtTest 1.0
51+import com.canonical.Oxide 1.0
52+import Oxide.testsupport 1.0
53+
54+Item {
55+ id: root
56+ TestWebView {
57+ id: webView
58+
59+ property bool htmlInputElementFocus: true;
60+ focus: true;
61+ messageHandlers: [
62+ ScriptMessageHandler {
63+ msgId: "FOCUS-STATE"
64+ contexts: [ "oxide://testutils/" ]
65+ callback: function(msg) {
66+ webView.htmlInputElementFocus = msg.payload;
67+ }
68+ }
69+ ]
70+ }
71+
72+ TestCase {
73+ name: "webview_focus"
74+ when: windowShown
75+
76+ function init() {
77+ webView.htmlInputElementFocus = true;
78+ }
79+
80+ function cleanupTestCase() {
81+
82+ }
83+
84+ function test_initial_focus() {
85+ compare(webView.activeFocus, true, "webView.activeFocus == true");
86+ }
87+
88+ function test_toggle_focus() {
89+ webView.url = "https://testsuite/tst_focus.html";
90+ verify(webView.waitForLoadSucceeded(),
91+ "Timed out waiting for successful load");
92+
93+ compare(webView.activeFocus, true, "webView.activeFocus == true");
94+ compare(webView.htmlInputElementFocus, true, "webview.htmlInputElementFocus == true");
95+
96+ webView.getTestApi().evaluateCode(
97+ "document.addEventListener(\"oxidefocusstate\", function(event) {
98+ oxide.sendMessage(\"FOCUS-STATE\", event.detail.status);
99+ });", true);
100+
101+ webView.focus = false;
102+ TestUtils.waitFor(function() { return webView.activeFocus == false; });
103+ compare(webView.activeFocus, false, "webView.activeFocus == false");
104+ TestUtils.waitFor(function() { return webView.htmlInputElementFocus == false; });
105+ compare(webView.htmlInputElementFocus, false, "webview.htmlInputElementFocus == false");
106+
107+ webView.focus = true;
108+ TestUtils.waitFor(function() { return webView.activeFocus == false; });
109+ compare(webView.activeFocus, true, "webView.activeFocus == true");
110+ TestUtils.waitFor(function() { return webView.htmlInputElementFocus == true; });
111+ compare(webView.htmlInputElementFocus, true, "webview.htmlInputElementFocus == true");
112+ }
113+ }
114+}

Subscribers

People subscribed via source and target branches

to all changes: