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

[Rui Tiago Cação Matos]

Hi,

Thanks for the update. I'm replying to both v4 and your reply to my
previous mail. Inline:

On Tue, Feb 2, 2016 at 2:33 PM, Jan Arne Petersen <janarne at gmail.com> wrote:
> +    <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>

This seems fine. But just to clarify. It's perfectly valid for an
application to never call the disable request even if it loses focus
on an enabled surface. Right?

> +    <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
> +       default hints (auto completion, auto correction, auto capitalization)
> +       should be assumed.

On Tue, Feb 2, 2016 at 1:26 PM, Jan Arne Petersen <janarne at gmail.com> wrote:
> Yes, they might be not relevant for all input methods. Removed the
> defaults,

They are still there though. I think that when unset this should
default to "normal" purpose and "none" hint.

> which should be handled by toolkits (what to set when nothing
> special is specified on an input widget).

Sure, toolkits, or clients in general, might always default on their
side to always call this request but that's beyond the protocol. What
the protocol needs to be clear about is what's assumed by the
compositor/IM when clients don't make this request.

In fact, that brings up another issue: _when_ does the compositor/IM
revert to the default purpose and hints? Does it revert when the text
input focus moves to a different wl_surface? Is it kept since it was
last set for all wl_surfaces of a particular client? Does it revert
when a client calls "update_state" ? There's several options here.

> +    <request name="set_cursor_rectangle">
> +      <description summary="set cursor position">
> +       Sets the cursor outline as a rectangle relative to the surface.
> +
> +       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"/>

These arguments could, and perhaps should, all be type="uint" .

> +    <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>

I think this request also needs to specify when the preferred language
reverts to the unset state.

> +    <enum name="update_state">
> +      <description summary="update_state flags">
> +       Defines the reason for sending an updated state.
> +      </description>
> +      <entry name="change" value="0" summary="updated state because it changed"/>
> +      <entry name="full" value="1" summary="full state after demand_full_state 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.
> +
> +       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.
> +      </description>
> +      <arg name="serial" type="uint" summary="used to identify the known state"/>
> +      <arg name="flags" type="uint" enum="update_state"/>
> +    </request>

I don't understand how the "flags" argument is useful. Can you give
examples of how it would change anything for the compositor/IM ?

I think this request should also specify all, if any, transient state
that should be considered reset after it. E.g. content hints and
purpose, preferred language, etc.

> +    <request name="invoke_action">
> +      <description summary="invokes an action">
> +       Allows to invoke an action when tapping or clicking the currently
> +       composed word in the text input.
> +
> +       This can be used by input methods to offer more word suggestions.
> +      </description>
> +      <arg name="button" type="uint"/>
> +      <arg name="index" type="uint"/>
> +    </request>

This is still underspecified. What is button and index? Is there a
list of actions missing? Can you provide us with the use case you have
in mind for this?

> +    <event name="enter">
> +      <description summary="enter event">
> +       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.

There might or might not be a keyboard focus. Sure, if a compositor
has both a keyboard focus and a text focus, it makes sense for the
compositor to set both together to the same surface but that's not
necessary and I don't see any reason why we should tangle them in the
protocol.

> +    <event name="modifiers_map">
> +    <event name="keysym">

Let's discuss these two together (btw, if they stay in the protocol,
they should be moved to be together IMO).

On Tue, Feb 2, 2016 at 1:26 PM, Jan Arne Petersen <janarne at gmail.com> wrote:
> There might be no hardware keyboard present (so no wl_keyboard
> interface). This is mostly useful for synthetic key events generated by
> a virtual keyboard (enter, backspace, cursor, whatever), which cannot be
> send via wl_keyboard.

Nothing prevents the compositor from exposing a wl_keyboard object
when there's no keyboard hardware and use it to send events generated
from a virtual keyboard.

Having these events here means that compositors would have to
duplicate to a great extent the code they already have to handle
wl_keyboard and just make things less clear in general.

If there's anything wl_keyboard doesn't allow us to do, by all means
let's add specific requests to this protocol. But these two requests
should be dropped IMO.

> +    <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 bounding rectangle on
> +       the surface with text focus which is overlapped by the input panel
> +       (virtual keyboard). That can be used to make widgets with text focus
> +       visible.
> +      </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>

All the coordinates could be uint. Also, I think we should add that
"compositors should send this event right after an enter event" since
the virtual keyboard might already be visible.

> +    <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 loosing focus).

Typo: "losing"

> +    <event name="demand_full_state">
> +      <description summary="demand the full state from client">
> +       Ask to get full current state information sent from the client via
> +       state requests (set_surrounding_text, set_content_hint, ...) and
> +       update_state.
> +      </description>
> +      <arg name="serial" type="uint" summary="serial of the latest known text input state"/>
> +      <arg name="flags" type="uint" summary="currently unused"/>
> +    </event>
> +  </interface>

Can you provide an example of when should a compositor/IM send this event?

> +  <interface name="zwp_text_input_manager_v2" version="1">
> +    <description summary="text input manager">
> +      A factory for text-input objects. This object is a global singleton.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="Destroy the wp_text_input_manager">
> +       Destroy the wp_text_input_manager object.
> +      </description>
> +    </request>

Given that this a global singleton it doesn't make sense to have a
destructor for it.

Thanks,
Rui