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

[Jan Arne Petersen]

Hi,

On 08/02/16 16:38, Rui Tiago Cação Matos wrote:
> 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?

Yes that is fine. The input method gets active then again next time the
surface gets text focus.

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

Yes exactly "normal" purpose and "none" hint would be the default.

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

Yes all state gets lost when a client calls update_state with a flag !=
change. After a new surface gets active input focus it must send an
updated state. I will add it to the documentation.

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

Yes I guess for width/height.

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

See above.

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

A update_state(change) request is used when just a specific state is
changed on client side (for example surrounding text).

Update_state(full) is used after a demand_full_state event is sent from
compositor/input method. All previous set state is lost. That event
might be send when an input method gets switched on compositor side and
thenew input method does not have any state yet.

Update_state(reset) is used when a client side widget keeps the input
method active but the current input gets interrupted (i.e. copy/pasting
into current text widget). All previous set state is lost (That seems to
be easier than to require from the client to send just changed state).

Update_state(enter is used when a new client side widget gets the active
input method (either after switching focus inside the surface on client
side, or by gaining text focus to a surface). All previous set state is
lost. This is the case mentioned above.

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

Yes in all cases except for update_state(change).

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

Yes indeed I will add more documentation.

It is used to be able to interact with pre-edit text (move the cursor
within pre-edit text or touch-and-hold to get some more options).

Button is button from wl_pointer::button event (or equivalent for
touch). Index the index within pre-edit text at which the click/touch
happened.

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

Yes we have the text focus in cases there is no keyboard available
(currently there is no way to have a focus then).

It does not make sense to have text and keyboard focus on different
surfaces at least not for the same seat. I do not see any case where you
would want hardware keyboard input and virtual keyboard input going into
different widgets/surfaces at the same time.

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

wl_keyboard currently does not allow to send synthetic key events when
you want to just send keysyms.

I am not sure if it really makes sense to extend wl_keyboard to support
that.

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

Fixed.

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

That would be mostly used when you switch input method (language) while
input is happening, so we do not need to cache the state in
compositor/input method framework.

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

Yes I will remove it.

> 
> Thanks,
> Rui
> 

-- 
Jan Arne Petersen | jan.petersen at kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts