Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Bill Spitzak]

On Tue, Feb 2, 2016 at 4:58 AM, Jan Arne Petersen  wrote:

>
>
> On 29/01/16 00:33, Bill Spitzak wrote:
> >
> >
> > On Wed, Jan 27, 2016 at 11:52 PM, Jan Arne Petersen  > > wrote:
> >
> >
> > +  Text is generally UTF-8 encoded, indices and lengths are in
> > bytes.
> >
> >
> > Remove the word "generally". *All* text in your api's are UTF-8.
>
> Done
>
> > +
> > +  
> > +   Requests to activate a surface for text input (typically
> when a
> > +   text entry in it gets focus).
> > +
> > +   There can only be one active surface per client and seat.
> > When surface is
> > +   null all surfaces of the client get deactivated.
> > +  
> >
> >
> > I think clients should be allowed to send activate more than once per
> > surface, it is to indicate the input focus switching between widgets.
> > This removes the need for another api to indicate the widget is
> > switching. Sending null for the surface, or a different event, would
> > indicate that the keyboard focus is no longer on a text input widget.
>
> We use enable/disable request now. That should be a bit easier to handle
> from client side.
>
> > +
> > +  
> > +   Should be called by an editor widget when the input state
> > should be
> > +   reset, for example after the text was changed outside of the
> > normal
> > +   input method flow.
> > +  
> > +
> >
> >
> > I believe this request can be replaced by redundant activate requests.
>
> It got integrated into update_state.
>
> > +
> > +  
> > +   Sets the plain surrounding text around the input position.
> > Text is
> > +   UTF-8 encoded. Cursor is the byte offset within the
> > +   surrounding text. Anchor is the byte offset of the
> > +   selection anchor within the surrounding text. If there is no
> > selected
> > +   text anchor is the same as cursor.
> > +  
> > +  
> > +  
> > +  
> > +
> >
> >
> > The anchor could be very far away from the cursor, much farther than any
> > small limit on the request size. I think this means the anchor position
> > could be negative or larger than the text length. Sending this without
> > clamping would be a good idea, the input method would then have a hint
> > where the anchor is, and it can clamp the value itself.
> >
> > If the client wants new text to replace the selected text, the text
> > between anchor+cursor will be deleted, and any input method decisions
> > would depend on the text outside the anchor..cursor range. This may be
> > larger than the allowed buffer size, so I think clients have to send the
> > text as though the selection was deleted.
> >
> > A client that does not want to replace the selected text could still
> > send an anchor, but then I am not clear if the input method can take
> > advantage of knowing what characters are selected to modify the results.
> > So it is possible the anchor is not needed at all.
>
> Ok I use an int for anchor now. It still make sense for an input method
> to know what is in the selection even when it gets replaced by new
> entered text.
>

I am unclear whether it is the input method's job to decide whether
characters replace the selection, or the client's job.

My concern is this:

- The input method has rules so that if the text before the cursor is "A"
and the user types 'x', you get "Ay". If the text is any other letter such
as "C" you get "Cx".

- Assume the text before the cursor is "An" where all the 'n' are
selected, and the length of the selection exceeds the size allowed by the
surrounding text request. Therefore the input method cannot know there is
an 'A' there.

- The user types 'x'. The input method cannot be aware that there is an A
there and the result will be "Ax", not the correct "Ay".

My proposed solution is that the client decides whether input replaces the
selection or is appended to it. If it replaces the selection, the text it
sends the input method is with the selection already deleted.

Other solution is that the above situation does not happen in real input
methods.

>
> > I think you might want non-zero bits to *disable* features. This allows
> > zero to be the default, and means that if some new correction is
> > invented in the future then they can default to on without you having to
> > modify the numerical value of default.
>
> Toolkits need to handle default on client side now.
>

I'm concerned that the value of "default" should not change. Assuming most
toolkits choose default, it should not vary depending on when the toolkit
was compiled.


> Yes you do not want to show already typed text on a virtual keyboard in
> a password field. Usually the keyboard would show nothing then, no
> reason to show dots/stars on the virtual keyboard.
>

I would assume it is considered better to use preedit than to have a
vi

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Bill Spitzak]

On Tue, Feb 2, 2016 at 5:02 AM, Jan Arne Petersen  wrote:

>
> > Besides allowing the user to return to a misspelled word, it may make
> > sense to return to a preedit. I'm not sure as I have no experience with
> > Asian input methods. But it does seem possible.
>
> Sorry no toolkit supports that. While the current proposed protocol can
> be implemented with current Qt, Gtk+ and EFL. Androids input method API
> is also quite similar to what is proposed here.
>

I'm sorry but I am familiar toolkits that allow the user to return to a
previously misspelled word and invoke the spelling corrector. My browser is
right now drawing squiqqly red lines under "toolkits" and "squiggly".

This means that the "incorrect" highlighting has to be preserved from the
preedit string. I think the easiest way to do this is to merge the commit
and preedit apis, so an input method can commit highlighting. This means it
can commit any preedit string. I guess the client can leave the
highlighting or remove it depending on the client limitations. Returning to
preedit can only work if the preedit state can be determined from the
characters alone, as there is no method for the client to send the
highlighting or previous preedit range to the input method.

I find it difficult to imagine an implementation that is capable of showing
the preedit string yet unable to commit it, so I don't see how any toolkit
could not support this, unless it cannot show spelling error highlight
unless the cursor is right next to them.

I may be misunderstanding how you intend spelling correction / completion
to work, however.

Delete surrounding text also works for pre-edit, i fixed the documentation.
>

I was curious how you intend for it to indicate a misspelling when the
input method is adding a space after a word that may have been typed
previously. Two methods I can think of are to have it delete the entire
word and replace it with the same word with the incorrect highlighting, or
to allow the incorrect highlighting range to be outside the preedit string
and include bytes already in the buffer.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

On 29/01/16 21:43, Bill Spitzak wrote:
> 
> 
> On Wed, Jan 27, 2016 at 11:52 PM, Jan Arne Petersen  > wrote:
> 
> 
> +
> +  
> +   Notify when a new composing text (pre-edit) should be set
> around the
> +   current cursor position. Any previously set composing text
> should
> +   be removed.
> +
> +   The commit text can be used to replace the preedit text on reset
> +   (for example on unfocus).
> +
> +   The text input should also handle all preedit_style and
> preedit_cursor
> +   events occurring directly before preedit_string.
> +  
> +  
> +  
> +  
> +
> +
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +
> +
> +  
> +   Sets styling information on composing text. The style is
> applied for
> +   length bytes from index relative to the beginning of the
> composing
> +   text (as byte offset). Multiple styles can
> +   be applied to a composing text by sending multiple
> preedit_styling
> +   events.
> +
> +   This event is handled as part of a following preedit_string
> event.
> +  
> +  
> +  
> +  
> +
> +
> +  
> +   Sets the cursor position inside the composing text (as byte
> +   offset) relative to the start of the composing text. When
> index is a
> +   negative number no cursor is shown.
> +
> +   This event is handled as part of a following preedit_string
> event.
> +  
> +  
> +
> +
> +  
> +   Notify when text should be inserted into the editor widget.
> The text to
> +   commit could be either just a single character after a key
> press or the
> +   result of some composing (pre-edit). It could be also an
> empty text
> +   when some text should be removed (see
> delete_surrounding_text) or when
> +   the input cursor should be moved (see cursor_position).
> +
> +   Any previously set composing text should be removed.
> +  
> +  
> +  
> +
> +
> +  
> +   Notify when the cursor or anchor position should be modified.
> +
> +   This event should be handled as part of a following
> commit_string
> +   event.
> +  
> +  
> +  
> +
> +
> +  
> +   Notify when the text around the current cursor position
> should be
> +   deleted.
> +
> +   Index is relative to the current cursor (in bytes).
> +   Length is the length of deleted text (in bytes).
> +
> +   This event should be handled as part of a following
> commit_string
> +   event.
> +  
> +  
> +  
> +
> 
> 
> I feel there are a lot of reasons to combine the preview and commit
> strings. An actual commit would be done by setting the preview and then
> sending a commit event.
> 
> - The preview has to be able to delete surrounding text, so that it can
> preview a merge between typed text and surrounding characters.
> 
> - Conversely, to match how most text input works in software now, you
> need to preserve the "incorrect" indicator in the committed text. This
> is so the word remains marked as incorrect. There also has to be a way
> for the client to return to that text and tell the input method to
> continue, so the spelling correction code in the input method can be
> reused (not just to make the client's work easier, but for the far more
> important reason of making the ui consistent!).
> 
> These remove the differences between commit and preview, so it makes
> sense to merge them.
> 
> Besides allowing the user to return to a misspelled word, it may make
> sense to return to a preedit. I'm not sure as I have no experience with
> Asian input methods. But it does seem possible.

Sorry no toolkit supports that. While the current proposed protocol can
be implemented with current Qt, Gtk+ and EFL. Androids input method API
is also quite similar to what is proposed here.

> So some possible modifications:
> 
> - Make delete surrounding text part of preview. Client has to remember
> the deleted text so it can restore it for the next preview (which may
> delete less).

Delete surrounding text also works for pre-edit, i fixed the documentation.

> - Change commit_string to just a commit event that says the previous
> preview string is the final result. This allows the commit to include
> highlighting.
> 
> - Possibly allow the highlight regions to be larger than the preview
> text. This could allow a spelling corrector to mark existing text as
> part of the incorrect word. The alternative is that the 

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

On 29/01/16 00:33, Bill Spitzak wrote:
> 
> 
> On Wed, Jan 27, 2016 at 11:52 PM, Jan Arne Petersen  > wrote:
> 
> 
> +  Text is generally UTF-8 encoded, indices and lengths are in
> bytes.
> 
> 
> Remove the word "generally". *All* text in your api's are UTF-8.

Done

> +
> +  
> +   Requests to activate a surface for text input (typically when a
> +   text entry in it gets focus).
> +
> +   There can only be one active surface per client and seat.
> When surface is
> +   null all surfaces of the client get deactivated.
> +  
> 
> 
> I think clients should be allowed to send activate more than once per
> surface, it is to indicate the input focus switching between widgets.
> This removes the need for another api to indicate the widget is
> switching. Sending null for the surface, or a different event, would
> indicate that the keyboard focus is no longer on a text input widget.

We use enable/disable request now. That should be a bit easier to handle
from client side.

> +
> +  
> +   Should be called by an editor widget when the input state
> should be
> +   reset, for example after the text was changed outside of the
> normal
> +   input method flow.
> +  
> +
> 
> 
> I believe this request can be replaced by redundant activate requests.

It got integrated into update_state.

> +
> +  
> +   Sets the plain surrounding text around the input position.
> Text is
> +   UTF-8 encoded. Cursor is the byte offset within the
> +   surrounding text. Anchor is the byte offset of the
> +   selection anchor within the surrounding text. If there is no
> selected
> +   text anchor is the same as cursor.
> +  
> +  
> +  
> +  
> +
> 
> 
> The anchor could be very far away from the cursor, much farther than any
> small limit on the request size. I think this means the anchor position
> could be negative or larger than the text length. Sending this without
> clamping would be a good idea, the input method would then have a hint
> where the anchor is, and it can clamp the value itself.
> 
> If the client wants new text to replace the selected text, the text
> between anchor+cursor will be deleted, and any input method decisions
> would depend on the text outside the anchor..cursor range. This may be
> larger than the allowed buffer size, so I think clients have to send the
> text as though the selection was deleted.
> 
> A client that does not want to replace the selected text could still
> send an anchor, but then I am not clear if the input method can take
> advantage of knowing what characters are selected to modify the results.
> So it is possible the anchor is not needed at all.

Ok I use an int for anchor now. It still make sense for an input method
to know what is in the selection even when it gets replaced by new
entered text.

> +  
> +  
> 
> 
> I think you might want non-zero bits to *disable* features. This allows
> zero to be the default, and means that if some new correction is
> invented in the future then they can default to on without you having to
> modify the numerical value of default.

Toolkits need to handle default on client side now.

> +  
> 
> 
> Can you explain this? The client still has to do something to display
> dots instead of characters, right? My guess is that this is an indicator
> that the input method should not show any of the surrounding text in
> popup controls. For instance maybe it can suggest word completion for
> your password, but the display must show stars for the already-typed
> characters. Is this correct?
> 
> Hopefully how clients and the input methods show hidden characters can
> be agreed by convention, rather than adding more api to communicate that.

Yes you do not want to show already typed text on a virtual keyboard in
a password field. Usually the keyboard would show nothing then, no
reason to show dots/stars on the virtual keyboard.

> +  
> 
> 
> Do you mean ASCII? It seems unlikely you mean "the subset of Unicode
> that they have declared are Latin characters" since that excludes
> numbers and space and includes vast numbers of characters that some
> software will not handle.

I guess we can remove it. Instead clients should just use
set_preferred_language("en").

> +  
> +  
> +  
> +  
> +  
> +  
> +  
> 
> 
> I think it will work a lot better if the client can indicate in
> set_surrounding_text that only a certain class of character is
> acceptable right now. So if it is a date of for nn/nn/nn, then it will
> indicate that slash or digit is required, depending on what is to the
> left of the cursor. And the meaning of "phone number" and "date" and
> "time" and "url" (and "zip code" and "ssn" and "inventor

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

Hi Rui,

On 28/01/16 21:16, Rui Tiago Cação Matos wrote:
> Hi,
> 
> thanks for sending this. I was actually going to start a discussion
> about the existing protocol but let's go from here instead. Comments
> inline:
> 
> First a question of scope: a client would only ever need to instance
> one text_input object per wl_seat and then "associate" it with a
> particular wl_surface through the activate request. Am I right?

Yes. There is one text_input object per wl_seat, which can be used to
track the text focus via enter/leave and to enable text input per
wl_suface via enable/disable (renamed in v3).

So there is only text input happening when the surface which has text
focus is also enabled.

> Not having a destroy request isn't so bad if that's the intended scope
> but I think we should still have one.

Added them.

> On Thu, Jan 28, 2016 at 8:52 AM, Jan Arne Petersen  wrote:
>> +  Text is generally UTF-8 encoded, indices and lengths are in bytes.
> 
> Why bytes instead of characters? It seems to me that using characters
> is less error prone since an offset in bytes might end up in the
> middle of an UTF-8 encoded character.

Yes text needs also to be valid UTF-8. Bytes seemed easier to use for
different toolkits with UTF-16 strings. But yes one could also use
characters if there is a big demand for it.

>> +  Serials are used to synchronize the state between the text input and
>> +  an input method. New serials are sent by the text input in the
>> +  commit_state request and are used by the input method to indicate
>> +  the known text input state in evsents like preedit_string, 
>> commit_string,
>> +  and keysym. The text input can then ignore events from the input 
>> method
>> +  which are based on an outdated state (for example after a reset).
> 
> Any particular reason for serials to be generated and controlled by
> clients? For the intended use case it seems ok but it's odd when
> compared with other serials in wayland.

Yes usually serials are used in Wayland on compositor side to make sure
that requests from clients are valid (setting a cursor in response to an
enter event for example).

Here we need to make sure that events from the compositor still make
sense for the client. In this case serials need to be generated on
client side.

>> +
>> +  
>> +   Requests input panels (virtual keyboard) to show.
>> +  
>> +
>> +
>> +  
>> +   Requests input panels (virtual keyboard) to hide.
>> +  
>> +
> 
> Do we really need these requests? Isn't the fact that a text input is
> currently active enough for the compositor/IM to decide on its own if
> it should show an input panel?

Added some more description. Basically when the virtual keyboard is
hidden by pressing some hide button (or sliding it away or whatever)
there needs to be a way for the client to request it again.

>> +
>> +  
>> +   Should be called by an editor widget when the input state should be
>> +   reset, for example after the text was changed outside of the normal
>> +   input method flow.
>> +  
>> +
> 
> Shouldn't this have a serial argument if we go with client controlled serials?
> 
> Otherwise reset could remain like this but adding specification to
> what happens with serials sent in events from this point onwards.

Removed reset. It is part of update_state now.

> 
>> +
>> +  
>> +   Sets the content purpose and content hint. While the purpose is the
>> +   basic purpose of an input field, the hint flags allow to modify some
>> +   of the behavior.
>> +
>> +   When no content type is explicitly set, a normal content purpose with
>> +   default hints (auto completion, auto correction, auto capitalization)
>> +   should be assumed.
> 
> No strong opinion, particularly since these are just hints and thus
> compositors/IMs are free to ignore them (right?), but I think the
> default should be "none".

Yes, they might be not relevant for all input methods. Removed the
defaults, which should be handled by toolkits (what to set when nothing
special is specified on an input widget).

>> +
>> +  
>> +  
>> +  
>> +  
>> +
> 
> This needs specification. Why is it a rectangle and what does it
> represent exactly? Are the coordinates relative to the activated
> wl_surface? What are compositors expected to do with it?

"Sets the cursor outline as a rectangle relative to the surface.

Allows the compositor to put a window with word suggestions near the
cursor." from patch v3

>> +
>> +  
>> +   Defines the reason for sending an updated state.
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>> +
>> +  
>> +  
>> +
> 
> This seems very underspecified. Care to elaborate how it's supposed to be 
> used?

See better description in v3:
"Allows to atomically send state updates from client.

 This request should follow after a batch of state updating request

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

Hi,

On 28/01/16 13:05, Daiki Ueno wrote:
> Hello,
> 
> Jan Arne Petersen  writes:
> 
>> Fixes some shortcomings of the first version:
>>
>> * Use only one wp_text_input per wl_seat (client side should be
>>   handled by client toolkit)
>> * Allow focus tracking without wl_keyboard present
> 
> These sound nice.
> 
>> * Improve update state handling
>> * Allow state requests
> 
> I have a few comments regarding them, inlined below.
> 
>> +  Serials are used to synchronize the state between the text input and
>> +  an input method. New serials are sent by the text input in the
>> +  commit_state request and are used by the input method to indicate
> 
> There is no commit_state request anymore.

Ok fixed to update_state.

>> +
>> +  
>> +Request to get the surrounding text and cursor position sent from the 
>> client.
>> +  
>> +  
>> +  
>> +  
>> +  
>> +
>> +
>> +  
>> +Request to get the surrounding text and cursor position sent from the 
>> client.
>> +  
>> +  
>> +  
>> +
> 
> These events have the same description.  Would you mind explaining how
> they are supposed to be used, maybe in combination with the
> set_surrounding_text request?

They are called configure_surrounding_text and demand_full_state and are
better documented in PATCH v3.

Thanks
Jan Arne

-- 
Jan Arne Petersen | jan.peter...@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

On 28/01/16 11:05, Jonas Ådahl wrote:
> On Thu, Jan 28, 2016 at 08:52:40AM +0100, Jan Arne Petersen wrote:
>> There were some shortcomings in the first version of the protocol which
>> makes it not really useful in real world applications. It is not really
>> possible to fix them in a compatible way so introduce a new v2 of the
>> protocol.
>>
>> Fixes some shortcomings of the first version:
>>
>> * Use only one wp_text_input per wl_seat (client side should be
>>   handled by client toolkit)
>> * Allow focus tracking without wl_keyboard present
>> * Improve update state handling
>> * Allow state requests
> 
> I haven't looked much at the actual protocol yet, but I have some
> comments anyway to begin with.
>> ---
>>  unstable/text-input/text-input-unstable-v2.xml | 369 
>> +
>>  1 file changed, 369 insertions(+)
>>  create mode 100644 unstable/text-input/text-input-unstable-v2.xml
>>
>> diff --git a/unstable/text-input/text-input-unstable-v2.xml 
>> b/unstable/text-input/text-input-unstable-v2.xml
>> new file mode 100644
>> index 000..f757e92
>> --- /dev/null
>> +++ b/unstable/text-input/text-input-unstable-v2.xml
>> @@ -0,0 +1,369 @@
>> +
>> +
>> +
>> +  
>> +Copyright © 2012, 2013 Intel Corporation
>> +Copyright © 2015, 2016 Jan Arne Petersen
>> +
>> +Permission to use, copy, modify, distribute, and sell this
>> +software and its documentation for any purpose is hereby granted
>> +without fee, provided that the above copyright notice appear in
>> +all copies and that both that copyright notice and this permission
>> +notice appear in supporting documentation, and that the name of
>> +the copyright holders not be used in advertising or publicity
>> +pertaining to distribution of the software without specific,
>> +written prior permission.  The copyright holders make no
>> +representations about the suitability of this software for any
>> +purpose.  It is provided "as is" without express or implied
>> +warranty.
>> +
>> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
>> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
>> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
>> +THIS SOFTWARE.
>> +  
>> +
>> +  
>> +
>> +  The zwp_text_input_v2 interface repesents text input and input methods
>> +  associated with a seat. It provides enter/leave event to follow the
>> +  text input focus for a seat.
>> +
>> +  Requests are used to activate/deactivate the text-input object and set
>> +  state information like surrounding and selected text or the content 
>> type.
>> +  The information about entered text is sent to the text-input object 
>> via
>> +  the pre-edit and commit events. Using this interface removes the need
>> +  for applications to directly process hardware key events and compose 
>> text
>> +  out of them.
>> +
>> +  Text is generally UTF-8 encoded, indices and lengths are in bytes.
>> +
>> +  Serials are used to synchronize the state between the text input and
>> +  an input method. New serials are sent by the text input in the
>> +  commit_state request and are used by the input method to indicate
>> +  the known text input state in evsents like preedit_string, 
>> commit_string,
>> +  and keysym. The text input can then ignore events from the input 
>> method
>> +  which are based on an outdated state (for example after a reset).
>> +
> 
> Please add an empty line between the events, requests and enums in the
> interfaces. It makes it much easer to read.
> 
> Seems to miss a 

Both done

>> +
>> +  
>> +Requests to activate a surface for text input (typically when a
>> +text entry in it gets focus).
>> +
>> +There can only be one active surface per client and seat. When surface 
>> is
>> +null all surfaces of the client get deactivated.
> 
> Why not make this a separate request and drop the allow-null? It makes
> it clearer that "activate" sometimes deactivates.

There are enable and disable now.

>> +  
>> +  > allow-null="true"/>
>> +
>> +
>> +  
>> +Requests input panels (virtual keyboard) to show.
>> +  
>> +
>> +
>> +  
>> +Requests input panels (virtual keyboard) to hide.
>> +  
>> +
>> +
>> +  
>> +Should be called by an editor widget when the input state should be
>> +reset, for example after the text was changed outside of the normal
>> +input method flow.
>> +  
>> +
>> +
>> +  
>> +Sets the plain surrounding text around the input position. Text is
>> +UTF-8 encoded. Cursor

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Rui Tiago Cação Matos]

Hi,

thanks for sending this. I was actually going to start a discussion
about the existing protocol but let's go from here instead. Comments
inline:

First a question of scope: a client would only ever need to instance
one text_input object per wl_seat and then "associate" it with a
particular wl_surface through the activate request. Am I right?

Not having a destroy request isn't so bad if that's the intended scope
but I think we should still have one.

On Thu, Jan 28, 2016 at 8:52 AM, Jan Arne Petersen  wrote:
> +  Text is generally UTF-8 encoded, indices and lengths are in bytes.

Why bytes instead of characters? It seems to me that using characters
is less error prone since an offset in bytes might end up in the
middle of an UTF-8 encoded character.

> +  Serials are used to synchronize the state between the text input and
> +  an input method. New serials are sent by the text input in the
> +  commit_state request and are used by the input method to indicate
> +  the known text input state in evsents like preedit_string, 
> commit_string,
> +  and keysym. The text input can then ignore events from the input method
> +  which are based on an outdated state (for example after a reset).

Any particular reason for serials to be generated and controlled by
clients? For the intended use case it seems ok but it's odd when
compared with other serials in wayland.

> +
> +  
> +   Requests input panels (virtual keyboard) to show.
> +  
> +
> +
> +  
> +   Requests input panels (virtual keyboard) to hide.
> +  
> +

Do we really need these requests? Isn't the fact that a text input is
currently active enough for the compositor/IM to decide on its own if
it should show an input panel?

> +
> +  
> +   Should be called by an editor widget when the input state should be
> +   reset, for example after the text was changed outside of the normal
> +   input method flow.
> +  
> +

Shouldn't this have a serial argument if we go with client controlled serials?

Otherwise reset could remain like this but adding specification to
what happens with serials sent in events from this point onwards.

> +
> +  
> +   Sets the content purpose and content hint. While the purpose is the
> +   basic purpose of an input field, the hint flags allow to modify some
> +   of the behavior.
> +
> +   When no content type is explicitly set, a normal content purpose with
> +   default hints (auto completion, auto correction, auto capitalization)
> +   should be assumed.

No strong opinion, particularly since these are just hints and thus
compositors/IMs are free to ignore them (right?), but I think the
default should be "none".

> +
> +  
> +  
> +  
> +  
> +

This needs specification. Why is it a rectangle and what does it
represent exactly? Are the coordinates relative to the activated
wl_surface? What are compositors expected to do with it?

> +
> +  
> +   Defines the reason for sending an updated state.
> +  
> +  
> +  
> +  
> +  
> +
> +
> +  
> +  
> +

This seems very underspecified. Care to elaborate how it's supposed to be used?

> +
> +  
> +  
> +

Underspecified.

> +
> +  
> +   Notification that this seat's text-input focus is on a certain 
> surface.
> +
> +   When the seat has one or more keyboards the text-input focus follows 
> the
> +   keyboard focus.

Wayland clients aren't aware of how many hardware keyboards exist. In
fact there might be none and a compositor is still free to advertise
the keyboard capability and assign keyboard focus (e.g. a touch screen
only device). I don't think we should mention wl_keyboard at all. Just
say that each wl_seat has a text-input focus and this is the enter
event for said focus.

> +
> +  
> +   Transfer an array of 0-terminated modifiers names. The position in
> +   the array is the index of the modifier as used in the modifiers
> +   bitmask in the keysym event.
> +  
> +  
> +

What's the purpose of this event?

> +
> +  
> +   Notify when the visibility state of the input panel changed.
> +  
> +  
> +

Unsure if we need this event at all. But if we do, it must at least
specify that this event will only be sent if this client currently
holds the text-input focus and the type should be be an enum.

> +
> +  
> +   Notify when a new composing text (pre-edit) should be set around the
> +   current cursor position. Any previously set composing text should
> +   be removed.

Should that read: "Any previously set _preedit_ text should be removed" ?

> +   The commit text can be used to replace the preedit text on reset

Can? I think this should be a "should" like below.

> +   The text input should also handle all preedit_style and preedit_cursor
> +   events o

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Daiki Ueno]

Hello,

Jan Arne Petersen  writes:

> Fixes some shortcomings of the first version:
>
> * Use only one wp_text_input per wl_seat (client side should be
>   handled by client toolkit)
> * Allow focus tracking without wl_keyboard present

These sound nice.

> * Improve update state handling
> * Allow state requests

I have a few comments regarding them, inlined below.

> +  Serials are used to synchronize the state between the text input and
> +  an input method. New serials are sent by the text input in the
> +  commit_state request and are used by the input method to indicate

There is no commit_state request anymore.

> +
> +  
> + Request to get the surrounding text and cursor position sent from the 
> client.
> +  
> +  
> +  
> +  
> +  
> +
> +
> +  
> + Request to get the surrounding text and cursor position sent from the 
> client.
> +  
> +  
> +  
> +

These events have the same description.  Would you mind explaining how
they are supposed to be used, maybe in combination with the
set_surrounding_text request?

Thanks,
-- 
Daiki Ueno
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

Hi,

On 28/01/16 10:46, Giulio Camuffo wrote:
>> +
>> +  
>> +   Sets the plain surrounding text around the input position. Text is
>> +   UTF-8 encoded. Cursor is the byte offset within the
>> +   surrounding text. Anchor is the byte offset of the
>> +   selection anchor within the surrounding text. If there is no selected
>> +   text anchor is the same as cursor.
>> +  
>> +  
> 
> The string type has a maximum allowed size, i wonder if that is a
> problem. See https://bugreports.qt.io/browse/QTBUG-43789
> If it's ok i think the documentation should be explicit about it,
> otherwise maybe we could pass a fd instead of a string, that the
> compositor would read from to get the surrounding text.

Yes, we should document, that one should not send too big texts with
this request. In my opinion 4076 Byte (around the cursor) should be
usually enough (also for performance reasons we do not want to send so
much text around). The limit needs to be enforced by the toolkit to
prevent bugs like the above though.

If there is rally a need for bigger surrounding texts, we can still add
another request with a fd to a later protocol version.

Regards
Jan Arne

-- 
Jan Arne Petersen | jan.peter...@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Jonas Ådahl]

On Thu, Jan 28, 2016 at 08:52:40AM +0100, Jan Arne Petersen wrote:
> There were some shortcomings in the first version of the protocol which
> makes it not really useful in real world applications. It is not really
> possible to fix them in a compatible way so introduce a new v2 of the
> protocol.
> 
> Fixes some shortcomings of the first version:
> 
> * Use only one wp_text_input per wl_seat (client side should be
>   handled by client toolkit)
> * Allow focus tracking without wl_keyboard present
> * Improve update state handling
> * Allow state requests

I haven't looked much at the actual protocol yet, but I have some
comments anyway to begin with.

> ---
>  unstable/text-input/text-input-unstable-v2.xml | 369 
> +
>  1 file changed, 369 insertions(+)
>  create mode 100644 unstable/text-input/text-input-unstable-v2.xml
> 
> diff --git a/unstable/text-input/text-input-unstable-v2.xml 
> b/unstable/text-input/text-input-unstable-v2.xml
> new file mode 100644
> index 000..f757e92
> --- /dev/null
> +++ b/unstable/text-input/text-input-unstable-v2.xml
> @@ -0,0 +1,369 @@
> +
> +
> +
> +  
> +Copyright © 2012, 2013 Intel Corporation
> +Copyright © 2015, 2016 Jan Arne Petersen
> +
> +Permission to use, copy, modify, distribute, and sell this
> +software and its documentation for any purpose is hereby granted
> +without fee, provided that the above copyright notice appear in
> +all copies and that both that copyright notice and this permission
> +notice appear in supporting documentation, and that the name of
> +the copyright holders not be used in advertising or publicity
> +pertaining to distribution of the software without specific,
> +written prior permission.  The copyright holders make no
> +representations about the suitability of this software for any
> +purpose.  It is provided "as is" without express or implied
> +warranty.
> +
> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +THIS SOFTWARE.
> +  
> +
> +  
> +
> +  The zwp_text_input_v2 interface repesents text input and input methods
> +  associated with a seat. It provides enter/leave event to follow the
> +  text input focus for a seat.
> +
> +  Requests are used to activate/deactivate the text-input object and set
> +  state information like surrounding and selected text or the content 
> type.
> +  The information about entered text is sent to the text-input object via
> +  the pre-edit and commit events. Using this interface removes the need
> +  for applications to directly process hardware key events and compose 
> text
> +  out of them.
> +
> +  Text is generally UTF-8 encoded, indices and lengths are in bytes.
> +
> +  Serials are used to synchronize the state between the text input and
> +  an input method. New serials are sent by the text input in the
> +  commit_state request and are used by the input method to indicate
> +  the known text input state in evsents like preedit_string, 
> commit_string,
> +  and keysym. The text input can then ignore events from the input method
> +  which are based on an outdated state (for example after a reset).
> +

Please add an empty line between the events, requests and enums in the
interfaces. It makes it much easer to read.

Seems to miss a 

> +
> +  
> + Requests to activate a surface for text input (typically when a
> + text entry in it gets focus).
> +
> + There can only be one active surface per client and seat. When surface 
> is
> + null all surfaces of the client get deactivated.

Why not make this a separate request and drop the allow-null? It makes
it clearer that "activate" sometimes deactivates.

> +  
> +   allow-null="true"/>
> +
> +
> +  
> + Requests input panels (virtual keyboard) to show.
> +  
> +
> +
> +  
> + Requests input panels (virtual keyboard) to hide.
> +  
> +
> +
> +  
> + Should be called by an editor widget when the input state should be
> + reset, for example after the text was changed outside of the normal
> + input method flow.
> +  
> +
> +
> +  
> + Sets the plain surrounding text around the input position. Text is
> + UTF-8 encoded. Cursor is the byte offset within the
> + surrounding text. Anchor is the byte offset of the
> + selection anchor within the surrounding text. If there is no selected
> + text anchor is the same as cursor.
> + 

Re: [PATCH wayland-protocols] text: Create second version of text input protocol

[Giulio Camuffo]

2016-01-28 9:52 GMT+02:00 Jan Arne Petersen :
> There were some shortcomings in the first version of the protocol which
> makes it not really useful in real world applications. It is not really
> possible to fix them in a compatible way so introduce a new v2 of the
> protocol.
>
> Fixes some shortcomings of the first version:
>
> * Use only one wp_text_input per wl_seat (client side should be
>   handled by client toolkit)
> * Allow focus tracking without wl_keyboard present
> * Improve update state handling
> * Allow state requests
> ---
>  unstable/text-input/text-input-unstable-v2.xml | 369 
> +
>  1 file changed, 369 insertions(+)
>  create mode 100644 unstable/text-input/text-input-unstable-v2.xml
>
> diff --git a/unstable/text-input/text-input-unstable-v2.xml 
> b/unstable/text-input/text-input-unstable-v2.xml
> new file mode 100644
> index 000..f757e92
> --- /dev/null
> +++ b/unstable/text-input/text-input-unstable-v2.xml
> @@ -0,0 +1,369 @@
> +
> +
> +
> +  
> +Copyright © 2012, 2013 Intel Corporation
> +Copyright © 2015, 2016 Jan Arne Petersen
> +
> +Permission to use, copy, modify, distribute, and sell this
> +software and its documentation for any purpose is hereby granted
> +without fee, provided that the above copyright notice appear in
> +all copies and that both that copyright notice and this permission
> +notice appear in supporting documentation, and that the name of
> +the copyright holders not be used in advertising or publicity
> +pertaining to distribution of the software without specific,
> +written prior permission.  The copyright holders make no
> +representations about the suitability of this software for any
> +purpose.  It is provided "as is" without express or implied
> +warranty.
> +
> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +THIS SOFTWARE.
> +  
> +
> +  
> +
> +  The zwp_text_input_v2 interface repesents text input and input methods
> +  associated with a seat. It provides enter/leave event to follow the
> +  text input focus for a seat.
> +
> +  Requests are used to activate/deactivate the text-input object and set
> +  state information like surrounding and selected text or the content 
> type.
> +  The information about entered text is sent to the text-input object via
> +  the pre-edit and commit events. Using this interface removes the need
> +  for applications to directly process hardware key events and compose 
> text
> +  out of them.
> +
> +  Text is generally UTF-8 encoded, indices and lengths are in bytes.
> +
> +  Serials are used to synchronize the state between the text input and
> +  an input method. New serials are sent by the text input in the
> +  commit_state request and are used by the input method to indicate
> +  the known text input state in evsents like preedit_string, 
> commit_string,
> +  and keysym. The text input can then ignore events from the input method
> +  which are based on an outdated state (for example after a reset).
> +
> +
> +  
> +   Requests to activate a surface for text input (typically when a
> +   text entry in it gets focus).
> +
> +   There can only be one active surface per client and seat. When 
> surface is
> +   null all surfaces of the client get deactivated.
> +  
> +   allow-null="true"/>
> +
> +
> +  
> +   Requests input panels (virtual keyboard) to show.
> +  
> +
> +
> +  
> +   Requests input panels (virtual keyboard) to hide.
> +  
> +
> +
> +  
> +   Should be called by an editor widget when the input state should be
> +   reset, for example after the text was changed outside of the normal
> +   input method flow.
> +  
> +
> +
> +  
> +   Sets the plain surrounding text around the input position. Text is
> +   UTF-8 encoded. Cursor is the byte offset within the
> +   surrounding text. Anchor is the byte offset of the
> +   selection anchor within the surrounding text. If there is no selected
> +   text anchor is the same as cursor.
> +  
> +  

The string type has a maximum allowed size, i wonder if that is a
problem. See https://bugreports.qt.io/browse/QTBUG-43789
If it's ok i think the documentation should be explicit about it,
otherwise maybe we could pass a fd instead of a string, that the
compositor would read from to get the surrounding text.


--
Giulio

> +  
> +

[PATCH wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

There were some shortcomings in the first version of the protocol which
makes it not really useful in real world applications. It is not really
possible to fix them in a compatible way so introduce a new v2 of the
protocol.

Fixes some shortcomings of the first version:

* Use only one wp_text_input per wl_seat (client side should be
  handled by client toolkit)
* Allow focus tracking without wl_keyboard present
* Improve update state handling
* Allow state requests
---
 unstable/text-input/text-input-unstable-v2.xml | 369 +
 1 file changed, 369 insertions(+)
 create mode 100644 unstable/text-input/text-input-unstable-v2.xml

diff --git a/unstable/text-input/text-input-unstable-v2.xml 
b/unstable/text-input/text-input-unstable-v2.xml
new file mode 100644
index 000..f757e92
--- /dev/null
+++ b/unstable/text-input/text-input-unstable-v2.xml
@@ -0,0 +1,369 @@
+
+
+
+  
+Copyright © 2012, 2013 Intel Corporation
+Copyright © 2015, 2016 Jan Arne Petersen
+
+Permission to use, copy, modify, distribute, and sell this
+software and its documentation for any purpose is hereby granted
+without fee, provided that the above copyright notice appear in
+all copies and that both that copyright notice and this permission
+notice appear in supporting documentation, and that the name of
+the copyright holders not be used in advertising or publicity
+pertaining to distribution of the software without specific,
+written prior permission.  The copyright holders make no
+representations about the suitability of this software for any
+purpose.  It is provided "as is" without express or implied
+warranty.
+
+THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
+AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
+ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
+THIS SOFTWARE.
+  
+
+  
+
+  The zwp_text_input_v2 interface repesents text input and input methods
+  associated with a seat. It provides enter/leave event to follow the
+  text input focus for a seat.
+
+  Requests are used to activate/deactivate the text-input object and set
+  state information like surrounding and selected text or the content type.
+  The information about entered text is sent to the text-input object via
+  the pre-edit and commit events. Using this interface removes the need
+  for applications to directly process hardware key events and compose text
+  out of them.
+
+  Text is generally UTF-8 encoded, indices and lengths are in bytes.
+
+  Serials are used to synchronize the state between the text input and
+  an input method. New serials are sent by the text input in the
+  commit_state request and are used by the input method to indicate
+  the known text input state in evsents like preedit_string, commit_string,
+  and keysym. The text input can then ignore events from the input method
+  which are based on an outdated state (for example after a reset).
+
+
+  
+   Requests to activate a surface for text input (typically when a
+   text entry in it gets focus).
+
+   There can only be one active surface per client and seat. When surface 
is
+   null all surfaces of the client get deactivated.
+  
+  
+
+
+  
+   Requests input panels (virtual keyboard) to show.
+  
+
+
+  
+   Requests input panels (virtual keyboard) to hide.
+  
+
+
+  
+   Should be called by an editor widget when the input state should be
+   reset, for example after the text was changed outside of the normal
+   input method flow.
+  
+
+
+  
+   Sets the plain surrounding text around the input position. Text is
+   UTF-8 encoded. Cursor is the byte offset within the
+   surrounding text. Anchor is the byte offset of the
+   selection anchor within the surrounding text. If there is no selected
+   text anchor is the same as cursor.
+  
+  
+  
+  
+
+
+  
+   Content hint is a bitmask to allow to modify the behavior of the text
+   input.
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  
+   The content purpose allows to specify the primary purpose of a text
+   input.
+
+   This allows an input method to show special purpose input panels with
+   extra characters or to disallow some characters.
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+
+  
+   Sets th