STEPS TO REPRODUCE: 1.Open Browser and Load http://waplabdc.nokia-boston.com/Browser/users/diana/enteralphabets.html 2.Begin entering text in the field given ACTUAL RESULTS: Accepts all the input characters EXPECTED RESULTS: Only alphabetical input should be accepted Linked to https://qtrequirements.europe.nokia.com/browse/BR-976
Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component
I can't reproduce this in QtLauncher - only alphabetical characters are accepted (and backspace of course). I notice firefox, konqueror and opera prevent backspace but that seems wrong to me.
Pretty sure webkit has nothing to do with this, but will reopen since I'm not testing on S60.
The problem is that a function defined to 'onKeyPress' never gets called. (<input type='text' onkeypress='validate(event)' />). With QtWebKit, InputBox gets its content via InputmethodEvents, not via keyevents.
(In reply to comment #4) > The problem is that a function defined to 'onKeyPress' never gets called. > (<input type='text' onkeypress='validate(event)' />). > > With QtWebKit, InputBox gets its content via InputmethodEvents, not via > keyevents. Are you sure? It seems to me that in QtLauncher the validate() function _is_ getting called and only allowing alphabetical characters as per the regex defined in the function. It also allows backspace/DEL but that seems a good thing. The test case is working fine so far as I can tell.
Yes, I'm sure. I tested it on N97 emulator and in N97 hw. This bug seems to exists with Symbian platform only.
(In reply to comment #4) > The problem is that a function defined to 'onKeyPress' never gets called. > (<input type='text' onkeypress='validate(event)' />). > > With QtWebKit, InputBox gets its content via InputmethodEvents, not via > keyevents. It might be possible to call onKeyPress for each char in the string returned from the input method event, however I guess that this would create other problems related to onKeyDown and onKeyUp not being called. What kind of problem would the end user encounter on a real website because of this limitation?
*** Bug 29372 has been marked as a duplicate of this bug. ***
(In reply to comment #8) > *** Bug 29372 has been marked as a duplicate of this bug. *** This seems like the same problem (if I understood well that S60 input is using the same channel as input method internationalization on other platforms) If I'm wrong please re-open the other bug. The other bug report also states that this works well in Safari and IE.
*** Bug 33166 has been marked as a duplicate of this bug. ***
Created attachment 54086 [details] patch file First attempt at patch. Tested on both Symbian and Linux platforms.
Attachment 54086 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/qt/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] WebKit/qt/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp" Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 54168 [details] patch file OK, let's try it again without the tab characters!
Are you using webkit-patch upload to post your patch? Why is the first one not getting obsoleted and its review flags cleared?
Comment on attachment 54086 [details] patch file I'll do a more thorough review tomorrow. Here are a few comments: > + //simulate keypress and keyrelease event for QWebpage to fire the javascript key event listeners > + //some input method can input more than 1 character at one time like Chinese input method, so a loop is needed. > + for (int i = 0; i < cmtedit.length(); ++i) { > + QChar echar = cmtedit.at(i); > + //if it's a ascii char, just transform it to key event to fire the javascript key event listeners QKeyEvents can be constructed with unicode characters. > + if (echar >= 0 && echar <= 255) { > + QKeyEvent* keypressevent = new QKeyEvent(QEvent::KeyPress, echar.toAscii(), Qt::NoModifier, echar); > + qApp->sendEvent(q, keypressevent); > + QKeyEvent* keyreleaseevent = new QKeyEvent(QEvent::KeyRelease, echar.toAscii(), Qt::NoModifier, echar); > + qApp->sendEvent(q, keyreleaseevent); I think you should allocate the key event on the stack instead of on the heap, to avoid leaking it. > + } else { > + //if it's an unicode char, insert it to the editor directly, since unicode char cannot be from a keyevent. > + editor->confirmComposition(QString(echar)); > + } > + } > + } else if (!ev->preeditString().isEmpty()) { > QString preedit = ev->preeditString(); > editor->setComposition(preedit, underlines, preedit.length(), 0); > }
Comment on attachment 54168 [details] patch file > Index: WebKit/qt/ChangeLog > =================================================================== > --- WebKit/qt/ChangeLog (revision 58169) > +++ WebKit/qt/ChangeLog (working copy) > @@ -1,3 +1,16 @@ > +2010-04-23 Tucker Jay <jay.tucker@nokia.com> > + > + Reviewed by NOBODY (OOPS!). > + > + [Qt] onKeyPress/Down/Up event handlers cannot be used when the text is entered through InputMethod > + https://bugs.webkit.org/show_bug.cgi?id=33165 > + > + Fixed bug preventing alpha-only and numeric-only text entry > + fields from working on Symbian platform. > + > + * Api/qwebpage.cpp: > + (QWebPagePrivate::inputMethodEvent): > + > 2010-04-22 John Pavan <john.pavan@nokia.com> > > Reviewed by Laszlo Gombos. > Index: WebKit/qt/Api/qwebpage.cpp > =================================================================== > --- WebKit/qt/Api/qwebpage.cpp (revision 58169) > +++ WebKit/qt/Api/qwebpage.cpp (working copy) > @@ -1188,9 +1188,26 @@ void QWebPagePrivate::inputMethodEvent(Q > } > } > > - if (!ev->commitString().isEmpty()) > - editor->confirmComposition(ev->commitString()); > - else if (!ev->preeditString().isEmpty()) { > + if (!ev->commitString().isEmpty()) { > + QString cmtedit = ev->commitString(); > + //delete the selection > + editor->confirmComposition(""); > + //simulate keypress and keyrelease event for QWebpage to fire the javascript key event listeners > + //some input method can input more than 1 character at one time like Chinese input method, so a loop is needed. > + for (int i = 0; i < cmtedit.length(); ++i) { > + QChar echar = cmtedit.at(i); > + //if it's a ascii char, just transform it to key event to fire the javascript key event listeners > + if (echar >= 0 && echar <= 255) { > + QKeyEvent* keypressevent = new QKeyEvent(QEvent::KeyPress, echar.toAscii(), Qt::NoModifier, echar); > + qApp->sendEvent(q, keypressevent); > + QKeyEvent* keyreleaseevent = new QKeyEvent(QEvent::KeyRelease, echar.toAscii(), Qt::NoModifier, echar); > + qApp->sendEvent(q, keyreleaseevent); > + } else { > + //if it's an unicode char, insert it to the editor directly, since unicode char cannot be from a keyevent. > + editor->confirmComposition(QString(echar)); > + } > + } > + } else if (!ev->preeditString().isEmpty()) { > QString preedit = ev->preeditString(); > editor->setComposition(preedit, underlines, preedit.length(), 0); > } WebKit/qt/Api/qwebpage.cpp:1193 + //delete the selection Please add a space after the comment sign WebKit/qt/Api/qwebpage.cpp:1194 + editor->confirmComposition(""); Please use String()'s default constructor instead of passing "", which does an implicit conversion from ascii and also allocate a StringImpl object on the heap for no reason. WebKit/qt/Api/qwebpage.cpp:1206 + //if it's an unicode char, insert it to the editor directly, since unicode char cannot be from a keyevent. This comment doesn't make sense to me. Why can't be a unicode char be sent from a key event? QKeyEvent takes a QString, and certainly there are keyboards out there that produce characters outside the ASCII range. WebKit/qt/Api/qwebpage.cpp:1201 + QKeyEvent* keypressevent = new QKeyEvent(QEvent::KeyPress, echar.toAscii(), Qt::NoModifier, echar); This QKeyEvent is allocated on the heap, but it is never deleted. Please allocate it on the stack instead. Please also add a test for this. For example you could write a unit test that generates synthetic input method events and before sending them initialize a snippet of JS that registers event listeners. Then you can verify if they were called correctly during the synthetic composition sequence or not.
Created attachment 54950 [details] revised patch Please take a look at the revised patch and the associated test case.
Comment on attachment 54950 [details] revised patch Please cancel the requested review. First of all, we've discovered that the test case being used (the test website) is itself flawed. More importantly, it's become apparent that it's not even sufficiently clear what the bug is and how it manifests itself, particularly on handheld devices.
(In reply to comment #18) > (From update of attachment 54950 [details]) > Please cancel the requested review. First of all, we've discovered that the test case being used (the test website) is itself flawed. More importantly, it's become apparent that it's not even sufficiently clear what the bug is and how it manifests itself, particularly on handheld devices. Is this still a valid bug report?
Yes, it is. It's still a topic of heated discussion. :-)
=== 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.