Bug 105618 – [EFL][WK2] Fix the position of cursor when handling preedit string

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[Seokju Kwon]

An input method may consume multiple key events in sequence and finally output the composed result.
(To input Chinese, Japanese, Korean and other complex languages).
Cursor position is incorrect when handling preedit string at this time.

[Seokju Kwon]

Created attachment 180514 [details]
Patch

[WebKit Review Bot]

Comment on attachment 180514 [details]
Patch

Attachment 180514 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15464324

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

[WebKit Review Bot]

Comment on attachment 180514 [details]
Patch

Attachment 180514 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15451475

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

[Seokju Kwon]

Created attachment 180587 [details]
Patch

[Ryuan Choi]

Comment on attachment 180587 [details]
Patch

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

LGTM without typo.

> Source/WebKit2/ChangeLog:3
> +        [EFL][WK2] Fix the postiion of cursor when handling preedit string

position

[Seokju Kwon]

Created attachment 180640 [details]
Patch

[Seokju Kwon]

(In reply to comment #5)
> (From update of attachment 180587 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180587&action=review
> 
> LGTM without typo.
> 
> > Source/WebKit2/ChangeLog:3
> > +        [EFL][WK2] Fix the postiion of cursor when handling preedit string
> 
> position

Fixed. Than you for your review.

[Seokju Kwon]

(In reply to comment #7)
> (In reply to comment #5)
> > (From update of attachment 180587 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=180587&action=review
> > 
> > LGTM without typo.
> > 
> > > Source/WebKit2/ChangeLog:3
> > > +        [EFL][WK2] Fix the postiion of cursor when handling preedit string
> > 
> > position
> 
> Fixed. Than you for your review.

Than -> Thank :)

[Laszlo Gombos]

Comment on attachment 180640 [details]
Patch

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

Overall looks good to me.

> Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:65
> +    ecore_imf_context_preedit_string_get(context, &buffer, &preeditPosition);

It would be great to validate preeditPosition (at least with ASSERT) after this call before using it later in the code.

[Seokju Kwon]

(In reply to comment #9)
> (From update of attachment 180640 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180640&action=review
> 
> Overall looks good to me.
> 
> > Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:65
> > +    ecore_imf_context_preedit_string_get(context, &buffer, &preeditPosition);
> 
> It would be great to validate preeditPosition (at least with ASSERT) after this call before using it later in the code.

Thanks, it is the position of cursor within the preedit string.
And I will add it after this call.

[Seokju Kwon]

Created attachment 180707 [details]
Patch

[Chris Dumez]

Comment on attachment 180707 [details]
Patch

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

> Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:74
> +    int cursorPosition = preeditString.length() + preeditPosition;

Why preeditString.length() + preeditPosition ? I thought preeditPosition was already the cursor position within the preedit string?

[Seokju Kwon]

(In reply to comment #12)
> (From update of attachment 180707 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180707&action=review
> 
> > Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:74
> > +    int cursorPosition = preeditString.length() + preeditPosition;
> 
> Why preeditString.length() + preeditPosition ? I thought preeditPosition was already the cursor position within the preedit string?

It could be 0. (in case of korean)

[Seokju Kwon]

(In reply to comment #12)
> (From update of attachment 180707 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180707&action=review
> 
> > Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:74
> > +    int cursorPosition = preeditString.length() + preeditPosition;
> 
> Why preeditString.length() + preeditPosition ? I thought preeditPosition was already the cursor position within the preedit string?

It is an offset into preedit string.
We need cursor position for the argument of setCompositon. (not cursor position within preedit string)

[Gyuyoung Kim]

Comment on attachment 180707 [details]
Patch

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

>>>> Source/WebKit2/UIProcess/efl/InputMethodContextEfl.cpp:74
>>>> +    int cursorPosition = preeditString.length() + preeditPosition;
>>> 
>>> Why preeditString.length() + preeditPosition ? I thought preeditPosition was already the cursor position within the preedit string?
>> 
>> It could be 0. (in case of korean)
> 
> It is an offset into preedit string.
> We need cursor position for the argument of setCompositon. (not cursor position within preedit string)

By the way, I wonder if cursorPosition can support meaningful functionality. Because, EFL port doesn't support *selectionEnd* parameter though cursorPosition means *selectionStart* in Editor.h.

In WebPageEfl.cpp, it sets last parameter with 0.

void WebPage::setComposition(const String& compositionString, const Vector<WebCore::CompositionUnderline>& underlines, uint64_t cursorPosition)
{
...
    targetFrame->editor()->setComposition(compositionString, underlines, cursorPosition, 0);


Shouldn't we support *selectionEnd* parameter as Qt and Mac ports ?

[Gyuyoung Kim]

Any update ?

[Seokju Kwon]

(In reply to comment #16)
> Any update ?

Oh. 
It seems that Ecore-imf doesn't fully support like other ports. So we can't get the other values from  ecore-imf APIs. 
Need to take a look. :)

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.