[PATCH wayland-protocols v7] text: Create second version of text input protocol
Jan Arne Petersen
janarne at gmail.com
Tue Jul 5 22:26:28 UTC 2016
On Thu, Jun 9, 2016 at 1:01 PM Carlos Garnacho <carlosg at gnome.org> wrote:
Hi Carlos,
Thanks for the feedback.
>
> Chiming in, and kinda late at that... hopefully we'll get this moving :).
I do not think we want to change text_input_unstable_v2 version 1 anymore
(since it already got shipped in Qt 5.7.0 and Plasma 5.7.0). Sorry for that.
But I already started to work on text_input_unstable_v2 version 2 and also
text_input_unstable_v3 where I like to include the feedback.
> First of all, I'm aware that some of my comments are directed towards
> stuff that's unchanged between v1 and v2, so please bear with me, I
> hope the feedback is useful.
Sure that is perfectly fine, I think that is the idea of the unstable protocols
anyways that we can still change everything and adapt them with real
world experience.
>
> On Mon, May 30, 2016 at 11:41 AM, Jan Arne Petersen <janarne at gmail.com> 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 and better define state handling
> > ---
> > Changes to v6:
> > * Fix some typos
> >
> > Makefile.am | 1 +
> > unstable/text-input/text-input-unstable-v2.xml | 480 +++++++++++++++++++++++++
> > 2 files changed, 481 insertions(+)
> > create mode 100644 unstable/text-input/text-input-unstable-v2.xml
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 71d2632..cc8d531 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -3,6 +3,7 @@ unstable_protocols = \
> > unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml \
> > unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml \
> > unstable/text-input/text-input-unstable-v1.xml \
> > + unstable/text-input/text-input-unstable-v2.xml \
> > unstable/input-method/input-method-unstable-v1.xml \
> > unstable/xdg-shell/xdg-shell-unstable-v5.xml \
> > unstable/relative-pointer/relative-pointer-unstable-v1.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 0000000..ea35d9b
> > --- /dev/null
> > +++ b/unstable/text-input/text-input-unstable-v2.xml
> > @@ -0,0 +1,480 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +
> > +<protocol name="text_input_unstable_v2">
> > + <copyright>
> > + 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.
> > + </copyright>
> > +
> > + <interface name="zwp_text_input_v2" version="1">
> > + <description summary="text input">
> > + The zwp_text_input_v2 interface represents text input and input methods
> > + associated with a seat. It provides enter/leave events to follow the
> > + text input focus for a seat.
> > +
> > + Requests are used to enable/disable the text-input object and set
> > + state information like surrounding and selected text or the content type.
> > + The information about the 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 valid UTF-8 encoded, indices and lengths are in bytes. Indices
> > + have to always point to the first byte of an UTF-8 encoded code point.
> > + Lengths are not allowed to contain just a part of an UTF-8 encoded code
> > + point.
> > +
> > + State is sent by the state requests (set_surrounding_text,
> > + set_content_type, set_cursor_rectangle and set_preferred_language) and
> > + an update_state request. After an enter or an input_method_change event
> > + all state information is invalidated and needs to be resent from the
> > + client. A reset or entering a new widget on client side also
> > + invalidates all current state information.
> > + </description>
> > +
> > + <request name="destroy" type="destructor">
> > + <description summary="Destroy the wp_text_input">
> > + Destroy the wp_text_input object. Also disables all surfaces enabled
> > + through this wp_text_input object
> > + </description>
> > + </request>
> > +
> > + <request name="enable">
> > + <description summary="enable text input for surface">
> > + Enable text input in a surface (usually when a text entry inside of it
> > + has focus).
> > +
> > + This can be called before or after a surface gets text (or keyboard)
> > + focus via the enter event. Text input to a surface is only active
> > + when it has the current text (or keyboard) focus and is enabled.
> > + </description>
> > + <arg name="surface" type="object" interface="wl_surface"/>
> > + </request>
>
> The "can be called before a surface gets text focus" bit concerns me a
> bit. Can this be called on several surfaces? Is the compositor
> expected to implement persistence? if that's the case, what's the
> benefit if only one surface can be active+focused at a given time?
Yes that is basically the idea, so that on focus switch between surfaces the
compositor can keep a virtual keyboard open when the newly focused surface
is also enabled/activated (instead of focus-out -> close keyboard -> focus-in ->
enable text input -> show keyboard). I am not so sure if that is so important
though.
> > +
> > + <request name="disable">
> > + <description summary="disable text input for surface">
> > + Disable text input in a surface (typically when there is no focus on any
> > + text entry inside the surface).
> > + </description>
> > + <arg name="surface" type="object" interface="wl_surface"/>
> > + </request>
> > +
> > + <request name="show_input_panel">
> > + <description summary="show input panels">
> > + Requests input panels (virtual keyboard) to show.
> > +
> > + This should be used for example to show a virtual keyboard again
> > + (with a tap) after it was closed by pressing on a close button on the
> > + keyboard.
> > + </description>
> > + </request>
>
> If I'm reading the spec correctly, it looks like the input panel can
> end up visible on two situations:
> 1) after the .enter event, and after a .enable request happened on the surface
> 2) after the user hid the keyboard (or the app requested
> .hide_input_panel) and .show_input_panel happens
>
> So, I wonder if this (and .hide_input_panel) can't be folded with the
> previous two calls :). In order to show the input panel as soon as a
> surface gets focus it'd require an explicit .enable/.show_input_panel
> call, but that IMHO fits well with the protocol in other places.
There is basically two modes here. Enabled without input panel/virtual
keyboard showing (still can get text events by other means like some
kind of hardware keyboard) and enabled and input panel/virtual keyboard
showing. There are definitely some embedded devices wanting both
modes.
> > +
> > + <request name="hide_input_panel">
> > + <description summary="hide input panels">
> > + Requests input panels (virtual keyboard) to hide.
> > + </description>
> > + </request>
> > +
> > + <request name="set_surrounding_text">
> > + <description summary="sets the surrounding text">
> > + 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.
> > +
> > + Make sure to always send some text before and after the cursor
> > + except when the cursor is at the beginning or end of text.
> > +
> > + When there was a configure_surrounding_text event take the
> > + before_cursor and after_cursor arguments into account for picking how
> > + much surrounding text to send.
> > +
> > + There is a maximum length of wayland messages so text can not be
> > + longer than 4000 bytes.
> > + </description>
> > + <arg name="text" type="string"/>
> > + <arg name="cursor" type="int"/>
> > + <arg name="anchor" type="int"/>
>
> Minor nitpick, would be great to have short summaries for arguments,
> esp. the ones that aren't self-explaining.
Yes I agree the documentation can still be improved.
> > + </request>
> > +
> > + <enum name="content_hint" bitfield="true">
> > + <description summary="content hint">
> > + Content hint is a bitmask to allow to modify the behavior of the text
> > + input.
> > + </description>
> > + <entry name="none" value="0x0" summary="no special behaviour"/>
> > + <entry name="auto_completion" value="0x1" summary="suggest word completions"/>
> > + <entry name="auto_correction" value="0x2" summary="suggest word corrections"/>
> > + <entry name="auto_capitalization" value="0x4" summary="switch to uppercase letters at the start of a sentence"/>
> > + <entry name="lowercase" value="0x8" summary="prefer lowercase letters"/>
> > + <entry name="uppercase" value="0x10" summary="prefer uppercase letters"/>
> > + <entry name="titlecase" value="0x20" summary="prefer casing for titles and headings (can be language dependent)"/>
> > + <entry name="hidden_text" value="0x40" summary="characters should be hidden"/>
> > + <entry name="sensitive_data" value="0x80" summary="typed text should not be stored"/>
> > + <entry name="latin" value="0x100" summary="just latin characters should be entered"/>
> > + <entry name="multiline" value="0x200" summary="the text input is multiline"/>
> > + </enum>
> > +
> > + <enum name="content_purpose">
> > + <description summary="content purpose">
> > + 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.
> > + </description>
> > + <entry name="normal" value="0" summary="default input, allowing all characters"/>
> > + <entry name="alpha" value="1" summary="allow only alphabetic characters"/>
> > + <entry name="digits" value="2" summary="allow only digits"/>
> > + <entry name="number" value="3" summary="input a number (including decimal separator and sign)"/>
> > + <entry name="phone" value="4" summary="input a phone number"/>
> > + <entry name="url" value="5" summary="input an URL"/>
> > + <entry name="email" value="6" summary="input an email address"/>
> > + <entry name="name" value="7" summary="input a name of a person"/>
> > + <entry name="password" value="8" summary="input a password (combine with password or sensitive_data hint)"/>
> > + <entry name="date" value="9" summary="input a date"/>
> > + <entry name="time" value="10" summary="input a time"/>
> > + <entry name="datetime" value="11" summary="input a date and time"/>
> > + <entry name="terminal" value="12" summary="input for a terminal"/>
> > + </enum>
> > +
> > + <request name="set_content_type">
> > + <description summary="set content purpose and hint">
> > + 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
> > + none hint should be assumed.
> > + </description>
> > + <arg name="hint" type="uint" enum="content_hint"/>
> > + <arg name="purpose" type="uint" enum="content_purpose"/>
> > + </request>
>
> It should be probably mentioned in this request that compositors are
> free to ignore hint, purpose, or invalid combinations of those. This
> probably means that it should be stated somewhere that clients should
> handle/ignore unexpected input too...
Yes that is indeed the case and even when a virtual keyboard just
allows to enter
digits after a content_type purpose set to digits the client might still have
some kind of entry for IPv4 addresses and ignore all digits invalid in
this context.
> > +
> > + <request name="set_cursor_rectangle">
> > + <description summary="set cursor position">
> > + Sets the cursor outline as a x, y, width, height rectangle in surface
> > + local coordinates.
> > +
> > + Allows the compositor to put a window with word suggestions near the
> > + cursor.
> > + </description>
> > + <arg name="x" type="int"/>
> > + <arg name="y" type="int"/>
> > + <arg name="width" type="int"/>
> > + <arg name="height" type="int"/>
> > + </request>
>
> These coordinates are relative to which surface? the one we last got
> the .enter event on? I have the feeling the surface should be an
> argument to this request, in order to ease checks in the compositors
> and remove ambiguity.
Yes local to the focused surface. I do not think it should be an issue
since there should be an update_state call from client side after a focus
change and old state should be dropped.
> > +
> > + <request name="set_preferred_language">
> > + <description summary="sets preferred language">
> > + Sets a specific language. This allows for example a virtual keyboard to
> > + show a language specific layout. The "language" argument is a RFC-3066
> > + format language tag.
> > +
> > + It could be used for example in a word processor to indicate language of
> > + currently edited document or in an instant message application which
> > + tracks languages of contacts.
> > + </description>
> > + <arg name="language" type="string"/>
> > + </request>
>
> I wonder... can this just be inferred from the desktop locale/current
> IM? Does it make a practical difference to the casual app launched
> with another LANG= to just use the .language request in order to force
> it?
There might be different text fields in the same application. Imagine an
chat application where you want to chat in different language to different
people (which is actually really annoying for example on Android currently,
but they added it to latest API version:
https://developer.android.com/reference/android/view/inputmethod/EditorInfo.html#hintLocales).
> > +
> > + <enum name="update_state">
> > + <description summary="update_state flags">
> > + Defines the reason for sending an updated state.
>
> I actually miss some further description of the fields here...
Yes documentation should be improved
> > + </description>
> > + <entry name="change" value="0" summary="updated state because it changed"/>
> > + <entry name="full" value="1" summary="full state after enter or input_method_changed event"/>
> > + <entry name="reset" value="2" summary="full state after reset"/>
> > + <entry name="enter" value="3" summary="full state after switching focus to a different widget on client side"/>
> > + </enum>
> > +
> > + <request name="update_state">
> > + <description summary="update state">
> > + Allows to atomically send state updates from client.
> > +
> > + This request should follow after a batch of state updating requests
> > + like set_surrounding_text, set_content_type, set_cursor_rectangle and
> > + set_preferred_language.
> > +
> > + The flags field indicates why an updated state is sent to the input
> > + method.
>
> The field is called "reason" below, and doesn't seem to be a flag set :).
Yes I will fix that.
> > +
> > + Reset should be used by an editor widget after the text was changed
> > + outside of the normal input method flow.
> > +
> > + For "change" it is enough to send the changed state, else the full
> > + state should be send.
>
> Oh, you explain the enum values here, somewhat :).
>
> > +
> > + Serial should be set to the serial from the last enter or
> > + input_method_changed event.
> > +
> > + To make sure to not receive outdated input method events after a
> > + reset or switching to a new widget wl_display_sync() should be used
> > + after update_state in these cases.
> > + </description>
> > + <arg name="serial" type="uint" summary="serial of the enter or input_method_changed event"/>
> > + <arg name="reason" type="uint" enum="update_state"/>
> > + </request>
>
> Ok... what is unclear to me (and should be at least documented) is the
> usefulness of the reason argument on both sides of the pipe. You sort
> of mention the situations when they should be expected to be emitted
> on the client, but there's no mentions as to what should the
> compositor do with these flags. Is it presumably expected to coalesce
> stuff or ignore unnecessary changes with these? if it's the latter
> case, should it ignore data received from a client request even if the
> flags don't match? Also, what's the minimal set of requests a client
> should perform prior to each of those reasons?
Yes documentation can be improved. Mandatory calls are just after
enter and input_method_changed events (with the full reason). It is
not really clear if state changes where still from the former focused
surface for example. So the compositor should ignore state changes
with the wrong reason.
Minimal requests is just an update_state() which would set all state to
default values.
> It also seems to me like some of those reason values are there to
> reaffirm the compositor, it already knows whether all state was
> dropped because of a focus switch, or if the input method changed.
Yes but compositor should make sure it does not get any old state
(from previous focused surface for example).
> However, it will send the respective events and the client must
> respond with the expected requests, plus this one with the
> corresponding expected flag. It sounds like extra room for
> misunderstandings. Otoh, there's basically one single reason to
> trigger this without compositor notice: focus changes inside the
> client.
Well there might be state changes, a new focused widget on client
side or a reset on the currently focused widget.
> All in all, if there's a set of requests that should be taking effect
> at once, IMHO it actually sounds best to have that data
> double-buffered. I realize that there previously was a "commit"
> request in v1 that was replaced by this one, however there's no
> mention in any of both versions to requests being honored in a delayed
> manner.
Yes that is the idea of update_state(). So all state is double buffered
and only takes effect on update_state(). I will improve documentation
there.
> > +
> > + <event name="enter">
> > + <description summary="enter event">
> > + Notification that this seat's text-input focus is on a certain surface.
> > +
> > + When the seat has the keyboard capability the text-input focus follows
> > + the keyboard focus.
> > + </description>
> > + <arg name="serial" type="uint" summary="serial to be used by update_state"/>
> > + <arg name="surface" type="object" interface="wl_surface"/>
> > + </event>
> > +
> > + <event name="leave">
> > + <description summary="leave event">
> > + Notification that this seat's text-input focus is no longer on
> > + a certain surface.
> > +
> > + The leave notification is sent before the enter notification
> > + for the new focus.
> > +
> > + When the seat has the keyboard capability the text-input focus follows
> > + the keyboard focus.
> > + </description>
> > + <arg name="serial" type="uint"/>
> > + <arg name="surface" type="object" interface="wl_surface"/>
> > + </event>
> > +
> > + <enum name="input_panel_visibility">
> > + <entry name="hidden" value="0"
> > + summary="the input panel (virtual keyboard) is hidden"/>
> > + <entry name="visible" value="1"
> > + summary="the input panel (virtual keyboard) is visible"/>
> > + </enum>
> > +
> > + <event name="input_panel_state">
> > + <description summary="state of the input panel">
> > + Notification that the visibility of the input panel (virtual keyboard)
> > + changed.
> > +
> > + The rectangle x, y, width, height defines the area overlapped by the
> > + input panel (virtual keyboard) on the surface having the text
> > + focus in surface local coordinates.
> > +
> > + That can be used to make sure widgets are visible and not covered by
> > + a virtual keyboard.
> > + </description>
> > + <arg name="state" type="uint" enum="input_panel_visibility"/>
> > + <arg name="x" type="int"/>
> > + <arg name="y" type="int"/>
> > + <arg name="width" type="int"/>
> > + <arg name="height" type="int"/>
> > + </event>
>
> I think I recognize the inspiration for this change :). However, do we
> really need passing the overlap coordinates in wayland? The compositor
> might well reconfigure the surface as it sees fit. Furthermore, the
> compositor already knows the current cursor position from the
> .set_cursor_rectangle request, so might do some smart placement as
> long as the surface size doesn't change (and thus does the cursor
> location)
>
> For fullscreen surfaces, I think it's better just have the compositor
> resize them in order to accommodate the input panel. This way things
> look fine even with clients that didn't commit full-on with text input
> v2.
>
> A compositor-side concern I have with this request is that it embeds
> UI state in the protocol. I'm aware that's partly the point, but some
> cases I can think of where this becomes ambiguous are:
> - input panes sliding from an edge. Presumably you can emit this event
> once per frame, but feels finicky...
> - split input pane (eg. osk in wide touchscreen in landscape position)
>
> As for client/toolkit-side concerns, tbh I've got quite a few :(...
> This model of keeping focus visible while typing only makes sense on a
> very certain set of UIs, mostly document editors, browsers, and others
> that still have plenty of room to relocate the focus on a visible
> area. It also pushes clients to handle all sorts of corner cases, are
> toolkits expected to handle overscrolling when the cursor is near the
> end of the document? what happens if the input panel covers entirely
> the focused text area? what happens if the input panel covers the
> entire client surface?
>
> IMHO, all those questions are better answered by having the compositor
> move/resize surfaces when the input panel is shown, we'd at least be
> using well tested mechanisms to keep the relevant bits of the UI in
> sight :), so I'd argue to remove this event altogether
Resize/Move is of course a valid way to handle virtual keyboards in a
compositor implementation and the compositor can just move/resize
surfaces and send input_panel_state(visible, 0, 0, 0, 0). But there
are also (embedded)
devices where surfaces should not get resized (because there is no layout
defined for a smaller height for example) and where the overlap information
is required.
What would be worth to add would maybe a kind of capability request where the
client (client toolkit) can specify it it supports overlapped keyboard
or rather wants to
always get resized from compositor.
> > +
> > + <event name="preedit_string">
> > + <description summary="pre-edit">
> > + 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 composing text in some cases
> > + (for example when losing focus).
> > +
> > + The text input should also handle all preedit_style and preedit_cursor
> > + events occurring directly before preedit_string.
> > + </description>
> > + <arg name="text" type="string"/>
> > + <arg name="commit" type="string"/>
> > + </event>
> > +
> > + <enum name="preedit_style">
> > + <entry name="default" value="0" summary="default style for composing text"/>
> > + <entry name="none" value="1" summary="composing text should be shown the same as non-composing text"/>
> > + <entry name="active" value="2" summary="composing text might be bold"/>
> > + <entry name="inactive" value="3" summary="composing text might be cursive"/>
> > + <entry name="highlight" value="4" summary="composing text might have a different background color"/>
> > + <entry name="underline" value="5" summary="composing text might be underlined"/>
> > + <entry name="selection" value="6" summary="composing text should be shown the same as selected text"/>
> > + <entry name="incorrect" value="7" summary="composing text might be underlined with a red wavy line"/>
> > + </enum>
> > +
> > + <event name="preedit_styling">
> > + <description summary="pre-edit styling">
> > + 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.
> > + </description>
> > + <arg name="index" type="uint"/>
> > + <arg name="length" type="uint"/>
> > + <arg name="style" type="uint" enum="preedit_style"/>
> > + </event>
>
> Is there any event that can be used to enclose the possible burst of
> .preedit_styling events? How does the client know when/whether no more
> of these events will be sent? In other protocols this is usually
> handled by a .done event, having a .preedit_done event or somesuch
> would be nice to have here.
It is called preedit_string here (that is even documented, see last sentence
of description ;-).
> > +
> > + <event name="preedit_cursor">
> > + <description summary="pre-edit cursor">
> > + 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.
> > +
> > + When no preedit_cursor event is sent the cursor will be at the end of
> > + the composing text by default.
> > +
> > + This event is handled as part of a following preedit_string event.
> > + </description>
> > + <arg name="index" type="int"/>
> > + </event>
> > +
> > + <event name="commit_string">
> > + <description summary="commit">
> > + 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.
> > + </description>
> > + <arg name="text" type="string"/>
> > + </event>
> > +
> > + <event name="cursor_position">
> > + <description summary="set cursor to new position">
> > + Notify when the cursor or anchor position should be modified. Cursor
> > + and anchor are set relative to the position at end of commit string
> > + (in bytes). Default is 0 for index and anchor.
> > +
> > + This event should be handled as part of a following commit_string
> > + event.
> > +
> > + The text between anchor and index should be selected.
> > + </description>
> > + <arg name="index" type="int" summary="position of cursor relative to end of inserted commit string"/>
> > + <arg name="anchor" type="int" summary="position of selection anchor relative to end of inserted commit string"/>
> > + </event>
>
> Is there no provision for cursor position changes without a string
> being committed? A case I can think of are terminals with an osk, you
> can't just tap anywhere, so an osk with the "terminal" hint might want
> to add cursor keys.
Yes you would just commit an empty string. I will update documentation.
> > +
> > + <event name="delete_surrounding_text">
> > + <description summary="delete surrounding text">
> > + Notify when the text around the current cursor position should be
> > + deleted. Before_length and after_length is the length (in bytes) of text
> > + before and after the current cursor position (excluding the selection)
> > + to delete.
> > +
> > + This event should be handled as part of a following commit_string
> > + or preedit_string event.
> > + </description>
> > + <arg name="before_length" type="uint" summary="length of text before current cursor positon"/>
> > + <arg name="after_length" type="uint" summary="length of text after current cursor positon"/>
>
> Typo, missing 'i' in "position" on both arguments.
I will fix that.
> > + </event>
> > +
> > + <event name="modifiers_map">
> > + <description summary="modifiers map">
> > + 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.
> > + </description>
> > + <arg name="map" type="array"/>
> > + </event>
>
> It should probably state that you'll receive this after the .enter event.
Well I would say it just happens after creating a text_input object actually.
But I think we can just have a fixed modifiers bitfield in zwp_text_input_v3
instead.
> > +
> > + <event name="keysym">
> > + <description summary="keysym">
> > + Notify when a key event was sent. Key events should not be used
> > + for normal text input operations, which should be done with
> > + commit_string, delete_surrounding_text, etc. The key event follows
>
> The wording here seems a bit off... what's the "key event" it
> mentions, this same event? in that case the first phrase reads a bit
> like "notifies about itself".
>
> It's also somewhat unclear to me what "normal text input operations"
> are, multichar input? any printable char? Should an OSK implementation
> use this event or the other ones? should it use both if it has both
> printable and non-printable (eg. tab) keys? IMHO there should be some
> examples about when to use which.
Yes the documentation should be updated. In the end each input method or
virtual keyboard implementation can decide if it prefers to use
delete_surrounding_text and commit_string or just keysym with
XKB_KEY_BackSpace for example. If a virtual keyboard just wants to send
simple key events and do not want to support any preedit text
or other advanced features there is no reason why to not just use keysym events
for all events in this case.
> > + the wl_keyboard key event convention. Sym is a XKB keysym, state a
>
> I wonder if it wouldn't be more consistent here if we used evcodes
> like wl_keyboard.key, instead of XKB keysyms. Having different pieces
> of the protocol talk slightly different conventions strikes me as odd.
Usually you do not have the evcode in a virtual keyboard but just a keysym and
it is not always possible to map that backwards (with what keymap when there
is not even a hardware keyboard?).
> > + wl_keyboard key_state. Modifiers are a mask for effective modifiers
> > + (where the modifier indices are set by the modifiers_map event)
> > + </description>
> > + <arg name="time" type="uint"/>
> > + <arg name="sym" type="uint"/>
> > + <arg name="state" type="uint"/>
> > + <arg name="modifiers" type="uint"/>
> > + </event>
> > +
> > + <event name="language">
> > + <description summary="language">
> > + Sets the language of the input text. The "language" argument is a RFC-3066
> > + format language tag.
> > + </description>
> > + <arg name="language" type="string"/>
> > + </event>
>
> Hmm, we've got a request and an event for this. Presumably the
> compositor sends this after .enter and after every .language request
> from the client? Are there situations where the compositor might want
> to have the last say? should the client shut up and follow? the
> bidirectionality here makes it a bit unclear on who takes precedence
> :).
The request is a hint by the client. The event is which language
(within a virtual keyboard
or an input method) the user really has selected (on virtual
keyboard/input method side).
Client might want to store it to set the language hint next time users
the same input field
for example.
I will update the documentation of the request and event to make it clearer.
> > +
> > + <enum name="text_direction">
> > + <entry name="auto" value="0" summary="automatic text direction based on text and language"/>
> > + <entry name="ltr" value="1" summary="left-to-right"/>
> > + <entry name="rtl" value="2" summary="right-to-left"/>
> > + </enum>
> > +
> > + <event name="text_direction">
> > + <description summary="text direction">
> > + Sets the text direction of input text.
> > +
> > + It is mainly needed for showing input cursor on correct side of the
> > + editor when there is no input yet done and making sure neutral
> > + direction text is laid out properly.
> > + </description>
> > + <arg name="direction" type="uint" enum="text_direction"/>
> > + </event>
>
> The description doesn't sound very convincing :)... Can't this be eg.
> inferred from the .language event or the locale?
Text direction depends on the script. Some languages have ltr or rtl
scripts. Since
the language might be selected by user explicitly in virtual
keyboard/input method
it is not cleat which locale to use.
> > +
> > + <event name="configure_surrounding_text">
> > + <description summary="configure amount of surrounding text to be sent">
> > + Configure what amount of surrounding text is expected by the
> > + input method. The surrounding text will be sent in the
> > + set_surrounding_text request on the following state information updates.
> > + </description>
> > + <arg name="before_cursor" type="int"/>
> > + <arg name="after_cursor" type="int"/>
> > + </event>
>
> Hmm, maybe "configure" is not the best term? this is a "reply me back
> with data" event, so perhaps "request" is better?
Yes I thought about request_surrounding_text first, but it also does
not request an immediate
reply but just tells the client how much text to send in all the following
set_surrounding_text requests.
> > +
> > + <event name="input_method_changed">
> > + <description summary="Notifies about a changed input method">
> > + The input method changed on compositor side, which invalidates all
> > + current state information. New state information should be sent from
> > + the client via state requests (set_surrounding_text,
> > + set_content_hint, ...) and update_state.
>
> IMO the expected requests perfom should be clearly stated here, i.e.
> no ellipsis.
I will update the documentation and list all state requests. But all
of them can be
omitted when the default value should be used for them.
> > + </description>
> > + <arg name="serial" type="uint" summary="serial to be used by update_state"/>
> > + <arg name="flags" type="uint" summary="currently unused"/>
>
> Won't argue hard against this, but after all this is an unstable
> protocol, this argument can be always added in future versions if it
> turns out needed :).
Indeed I will drop it in zwp_text_input_v3
Thanks for all the feedback.
Jan Arne
More information about the wayland-devel
mailing list