Bug 38685 - [Qt] tst_QWebPage::inputMethods failing on Maemo5
Summary: [Qt] tst_QWebPage::inputMethods failing on Maemo5
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Diego Gonzalez
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 38654
  Show dependency treegraph
 
Reported: 2010-05-06 13:34 PDT by Diego Gonzalez
Modified: 2010-05-14 01:06 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.66 KB, patch)
2010-05-07 15:15 PDT, Diego Gonzalez
kenneth: review-
Details | Formatted Diff | Diff
Patch (2.21 KB, patch)
2010-05-07 15:35 PDT, Diego Gonzalez
hausmann: review-
Details | Formatted Diff | Diff
Patch (2.87 KB, patch)
2010-05-11 12:23 PDT, Diego Gonzalez
kenneth: review-
Details | Formatted Diff | Diff
Patch (3.25 KB, patch)
2010-05-11 12:49 PDT, Diego Gonzalez
kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
Patch OK (3.28 KB, patch)
2010-05-11 14:27 PDT, Diego Gonzalez
diegohcg: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Gonzalez 2010-05-06 13:34:30 PDT
Test failing in Maemo 5 with Qt4.6

FAIL!  : tst_QWebPage::inputMethods(QWebView) '!viewEventSpy.contains(QEvent::RequestSoftwareInputPanel)' returned FALSE. ()
   Loc: [/home/diegohcg/webkit/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp(1370)] 
FAIL!  : tst_QWebPage::inputMethods(QGraphicsWebView) '!viewEventSpy.contains(QEvent::RequestSoftwareInputPanel)' returned FALSE. ()
   Loc: [/home/diegohcg/webkit/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp(1370)]
Comment 1 Diego Gonzalez 2010-05-07 15:12:23 PDT
The test checks if there is no RequestSoftwareInputPanel event in the EventSpy, when the input form receives a mouse click. 

QVERIFY(!viewEventSpy.contains(QEvent::RequestSoftwareInputPanel));

As the RequestSoftwareInputPanel should invoke a panel like a virtual keyboard, in Maemo it seems be called as it is a mobile platform. So, for the Maemo's case, the behavior of this the should be changed the the opposite. 

QVERIFY(viewEventSpy.contains(QEvent::RequestSoftwareInputPanel));

Maybe this test should also fail in another mobile platform like s60, but I didn't test it.
Comment 2 Diego Gonzalez 2010-05-07 15:15:10 PDT
Created attachment 55422 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2010-05-07 15:16:28 PDT
Laszlo, can you verify?
Comment 4 Kenneth Rohde Christiansen 2010-05-07 15:17:27 PDT
Comment on attachment 55422 [details]
Patch

WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1368
 +  #ifndef Q_WS_MAEMO_5
It would really be good with a comment here why there are different results, r- because of that. That should ALSO be in the ChangeLog.
Comment 5 Diego Gonzalez 2010-05-07 15:35:31 PDT
Created attachment 55423 [details]
Patch

Added comments
Comment 6 Simon Hausmann 2010-05-07 15:51:49 PDT
Comment on attachment 55423 [details]
Patch

So this part of the test checks that the SIP is triggered not on the first mouse click but on the _second_, corresponding to style->styleHint(...::RequestSoftwareInputPanel) != RSIP_OnMouseClick.

See QWebPagePrivate::handleSoftwareInputPanel

So the auto-test should probably adjust its expected result - either RSIP on first or second click - depending on the return value of the styleHint() function on the current style.

Then the auto-test could be written without platform #ifdefs :)
Comment 7 Diego Gonzalez 2010-05-10 09:17:10 PDT
(In reply to comment #6)
> (From update of attachment 55423 [details])
> So this part of the test checks that the SIP is triggered not on the first mouse click but on the _second_, corresponding to style->styleHint(...::RequestSoftwareInputPanel) != RSIP_OnMouseClick.
> 
> See QWebPagePrivate::handleSoftwareInputPanel
> 
> So the auto-test should probably adjust its expected result - either RSIP on first or second click - depending on the return value of the styleHint() function on the current style.
> 
> Then the auto-test could be written without platform #ifdefs :)

Yeah sure, it's make more sense. Thanks :)
Comment 8 Diego Gonzalez 2010-05-11 12:23:06 PDT
Created attachment 55735 [details]
Patch

Check if SIP is triggered via styleHint
Comment 9 Kenneth Rohde Christiansen 2010-05-11 12:29:57 PDT
Comment on attachment 55735 [details]
Patch


WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1371
 +      int  inputPanel= 0;
wrong coding style

WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1383
 +          QVERIFY(!viewEventSpy.contains(QEvent::RequestSoftwareInputPanel));
As said earlier, can be please add a comment here explaining the difference
Comment 10 Diego Gonzalez 2010-05-11 12:49:54 PDT
Created attachment 55739 [details]
Patch

added more comments
Comment 11 Kenneth Rohde Christiansen 2010-05-11 13:01:32 PDT
Comment on attachment 55739 [details]
Patch

WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1384
 +      // part of the test can verified as the checks bellow.
below

WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1381
 +      // because there is no SIP (Software Input Panel) triggered. In case of mobile
In the case of a mobile platform, AN input panel

WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1382
 +      // platforms a input panel, e.g. virtual keyboard, is normally invoked and the
usually might be better than normally

WebKit/qt/ChangeLog:10
+         Check if the SIP (Software Input Panel) is triggered,
+         which normally happens on mobile platforms, when a user input form receives
+         a mouse click.

Strange word wrap
Comment 12 Diego Gonzalez 2010-05-11 14:27:47 PDT
Created attachment 55756 [details]
Patch OK

Adjust comments in the already r+ patch
Comment 13 Diego Gonzalez 2010-05-13 12:38:12 PDT
Landed manually at r59380.
Comment 14 Simon Hausmann 2010-05-13 15:06:48 PDT
<cherry-pick-for-backport: r59380>
Comment 15 Simon Hausmann 2010-05-14 01:06:36 PDT
Revision r59380 cherry-picked into qtwebkit-2.0 with commit f5e95b4fdb7411ca7fbdb7ac1552acd47741e100