QWebPagePrivate::inputMethodEvent has 2 problems when compositing Korean. 1. The caret disappear after compositing Korean. 2. If an user types Korean until end of the text field, the border of the text field hides the compositing text, because of the caret prior to compositing text, not next to that.
Created attachment 76729 [details] [Qt] QWebPagePrivate::inputMethodEvent has problem when compositing Korean
Attachment 76729 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/qt/Api/qwebpage.cpp', u'WebKit/qt/ChangeLog']" exit_code: 1 WebKit/qt/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
I amended two points. First, added frame->selection()->setCaretVisible(true); It is because the caret should be visible on and after Korean composition (and the other languages that need composition.). Second, amend from editor->setComposition(ev->preeditString(), underlines, 0, 0) to editor->setComposition(ev->preeditString(), underlines, 1, 1). It is because it is more natural for the caret to be next to the composition word, not in front of the composition word. Moreover, the caret in front of the composition word makes the composition word on right end of the text field hide back text field box.
Created attachment 76730 [details] [Qt] QWebPagePrivate::inputMethodEvent has problem when compositing Korean
Comment on attachment 76730 [details] [Qt] QWebPagePrivate::inputMethodEvent has problem when compositing Korean How do we test this?
Comment on attachment 76730 [details] [Qt] QWebPagePrivate::inputMethodEvent has problem when compositing Korean View in context: https://bugs.webkit.org/attachment.cgi?id=76730&action=review How can we test this? > WebKit/qt/Api/qwebpage.cpp:1098 > + // that the caret is vanished. after finishing composition, caret should > + // be visible. Grammar. When compositing Korean, 'ev' is of type Cursor with length=0. This causes the caret to vanish. After the composition finishes, the caret should be visible. > WebKit/qt/Api/qwebpage.cpp:1102 > + // If preeditSting exists, Caret should be visible. preeditSting -> preeditString Caret -> the caret > WebKit/qt/Api/qwebpage.cpp:1105 > + // next to composition, not in front of composition. next to composition -> next to the composition in front of composition -> in front of the composition
Thank you for comment. It is necessary to check whether this patch is vaild because It is very old patch. If this is vaild, I'll update the patch with layout test.
Created attachment 91874 [details] Patch This patch is about the caret disappearing. I give up patching about the caret position on composition because when I patch I have no confidence whether the other language have valid caret position. If I have confidence I will patch again. Although I find the way how to test, I can not find the feasible test way. It is because 1. There is no JSObject API whether the caret is visible for layout test. 2. textInputController.setMarkedText's event is different from actual input method event. In detail about #2, the QInputMethodEvent instance that QWebPagePrivate::inputMethodEvent method receives includes two attribute types such as QInputMethodEvent::TextFormat and QInputMethodEvent::Cursor. However, textInputController.setMarkedText(,,) only create QInputMethodEvent::TextFormat type QInputMethodEvent instance. I don't know it is Qt specific or not.
Created attachment 91877 [details] patch I made mistake not to include ChangeLog.
Created attachment 91919 [details] Patch
I am waiting that someone reviews this patch.
Comment on attachment 91919 [details] Patch I would want a commetn there like "In some languages like Korean you can have a length of 0 after compositing, so the cursor should stay visible" or similar
Why should cursor be invisible if length is 0? Who does know the reason?
(In reply to comment #13) > Why should cursor be invisible if length is 0? > Who does know the reason? I thought that is what you patch does and you needed that for Korean. I do not know Korean, but it shows that you need to explain better why you are making the change then.
Created attachment 95382 [details] Patch I changed the approach. Qt document said QInputMethodEvent::Cursor's zero length means invisible. http://web.mit.edu/qt-dynamic/www/qinputmethodevent.html I think it make sense that the caret is invisible when compositing single text that the underline marks compositing here. If multi-texts are being composited like Arabic, it is necessary for caret to mark where is compositing. Although the caret became invisible due to composition, the caret should be visible after composition.
Comment on attachment 91919 [details] Patch Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 91919 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 95382 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95382&action=review > Source/WebKit/qt/Api/qwebpage.cpp:1133 > editor->confirmComposition(ev->commitString()); > else > editor->insertText(ev->commitString(), 0); > + frame->selection()->setCaretVisible(true); > } else if (!hasSelection && !ev->preeditString().isEmpty()) > editor->setComposition(ev->preeditString(), underlines, 0, 0); > - else if (ev->preeditString().isEmpty() && editor->hasComposition()) > + else if (ev->preeditString().isEmpty() && editor->hasComposition()) { > editor->confirmComposition(String()); > + frame->selection()->setCaretVisible(true); > + } > It should be possible to test this in tst_qwebpage. See void tst_QWebFrame::inputFieldFocus() for an example of testing for a blinking caret. > Source/WebKit/qt/ChangeLog:12 > + It is nature to composite the text without the caret if Cursor's length is zero. s/nature/natural
Comment on attachment 95382 [details] Patch r- for lack of testing per Robert's comment.
=== 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.