Fix text input bugs with grabbed popups and bugs found by new tests

* 12to11-test.xml (test_manager) <error>: Add
invalid_user_time.
<get_serial, serial> New request and event.
(test_seat_controller) <set_last_user_time>: New request.

* compositor.h (struct _TextInputFuncs): Make `filter_input'
return keycode.
* data_device.c (DestroyReference): Check if reference device is
detached before unlinking reference.
* seat.c (LookupKeysym): Delete function.
(DispatchKey): Use keycodes instead.
(XLSeatExplicitlyGrabSurface): Avoid using owner-events keyboard
grab.

* select.c (struct _SelectionOwnerInfo, HandleSelectionRequest):
Allow CurrentTime in selection requests.  Compensate for
wraparound as well.
(OwnSelection, DisownSelection): Use Timestamp instead of Time.
* test.c (GetSerial): New function.
(test_manager_impl): Add new function.
* test_seat.c (SetLastUserTime): New function.
(seat_controller_impl): Add new function.
* tests/Imakefile (LOCAL_LIBRARIES): Remove GBM and DRM.
(SRCS10, OBJS10, SRCS11, OBJS11): New variables.
(dmabuf_test): Only link this program with GBM and DRM.
(PROGRAMS): Add select_test, select_helper.
(select_test, select_helper): New programs.

* tests/README: Document that select_test needs to be run under
vfb.
* tests/run_tests.sh: Add TODO note.
* tests/svnignore.txt: Add select_test and select_helper.
* tests/test_harness.c (handle_test_manager_serial): New
function.
(test_manager_listener): Add new function.
(open_test_display): Clear display->seat.
(test_get_serial): New function.

* tests/test_harness.h (struct test_display): New function
`serial'.

* text_input.c (CreateIC): Improve debugging code.
(CalculateKeycodeForEvent): Move earlier.
(FilterInputCallback): Handle keycodes here as well.
(XLTextInputDispatchCoreEvent): Add more debugging code.

* xdata.c (SelectSelectionInput): Obtain server time here.
(XLOwnDragSelection, NoteLocalSelectionFooter): Convert times to
Timestamp.
This commit is contained in:
hujianwei 2022-11-12 03:51:21 +00:00
parent be955e3c79
commit 0bdc502068
15 changed files with 207 additions and 106 deletions

View file

@ -53,6 +53,8 @@
summary="the specified artifical device already exists"/>
<entry name="invalid_label" value="11"
summary="the specified label is invalid"/>
<entry name="invalid_user_time" value="12"
summary="the specified user time lies in the past"/>
</enum>
<request name="get_test_surface">
@ -102,6 +104,12 @@
<arg name="id" type="new_id" interface="test_seat_controller"/>
</request>
<request name="get_serial">
<description summary="obtain serial">
Send a serial event with the next serial of the display.
</description>
</request>
<event name="display_string">
<description summary="X server name">
The display_string event sends the name of the X display to
@ -110,6 +118,14 @@
</description>
<arg name="name" type="string"/>
</event>
<event name="serial">
<description summary="display serial">
The serial event is sent in reply to the get_serial request.
It provides the next serial that will be used by the display.
</description>
<arg name="serial" type="uint"/>
</event>
</interface>
<interface name="test_surface" version="1">
@ -581,6 +597,21 @@
<arg name="id" type="new_id" interface="test_device_controller"/>
</request>
<request name="set_last_user_time">
<description summary="set seat user time">
Set the user time of the specified seat to the specified time.
If the last user time is greater than the specified time,
raise an invalid_user_time error.
The month describes the number of times the millisecond
counter has wrapped around, and milliseconds specifies the X
server time. (This representation is also internally used in
the X sample server.)
</description>
<arg name="months" type="uint"/>
<arg name="milliseconds" type="uint"/>
</request>
<event name="device_id">
<description summary="device ID">
This event is sent immediately after the test_seat_controller

View file

@ -1544,8 +1544,8 @@ struct _TextInputFuncs
void (*focus_in) (Seat *, Surface *);
void (*focus_out) (Seat *);
/* The last argument is actually an XIDeviceEvent *. */
Bool (*filter_input) (Seat *, Surface *, void *, KeySym *);
/* The second-last argument is actually an XIDeviceEvent *. */
Bool (*filter_input) (Seat *, Surface *, void *, KeyCode *);
};
extern void XLTextInputDispatchCoreEvent (Surface *, XEvent *);
@ -1654,8 +1654,8 @@ extern unsigned char *ReadChunk (ReadTransfer *, int, ptrdiff_t *,
extern void SkipChunk (ReadTransfer *);
extern void CompleteDelayedTransfer (ReadTransfer *);
extern Bool OwnSelection (Time, Atom, GetDataFunc (*) (WriteTransfer *,
Atom, Atom *),
extern Bool OwnSelection (Timestamp, Atom, GetDataFunc (*) (WriteTransfer *,
Atom, Atom *),
Atom *, int);
extern void DisownSelection (Atom);

View file

@ -858,8 +858,14 @@ SendDataOffers (void)
static void
DestroyReference (DataDeviceReference *reference)
{
reference->next->last = reference->last;
reference->last->next = reference->next;
/* If reference->device is NULL, then the data device itself has
been destroyed. */
if (reference->device)
{
reference->next->last = reference->last;
reference->last->next = reference->next;
}
XLFree (reference);
}

52
seat.c
View file

@ -4308,30 +4308,10 @@ DispatchButton (Subcompositor *subcompositor, XIDeviceEvent *xev)
xev->event_x, xev->event_y);
}
static KeySym
LookupKeysym (XIDeviceEvent *xev, KeyCode keycode)
{
unsigned int state, mods_return;
KeySym keysym;
/* Convert the state to a core state mask. */
state = ((xev->mods.effective & ~(1 << 13 | 1 << 14))
| (xev->group.effective << 13));
keysym = 0;
/* Look up the keysym. */
XkbLookupKeySym (compositor.display, keycode,
state, &mods_return, &keysym);
/* Return the keysym. */
return keysym;
}
static void
DispatchKey (XIDeviceEvent *xev)
{
Seat *seat;
KeySym keysym;
KeyCode keycode;
seat = XLLookUpAssoc (seats, xev->deviceid);
@ -4345,12 +4325,12 @@ DispatchKey (XIDeviceEvent *xev)
if (seat->focus_surface)
{
keysym = 0;
keycode = 0;
if (input_funcs
&& seat->flags & IsTextInputSeat
&& input_funcs->filter_input (seat, seat->focus_surface,
xev, &keysym))
xev, &keycode))
/* The input method decided to filter the key. */
return;
@ -4358,25 +4338,9 @@ DispatchKey (XIDeviceEvent *xev)
if (xev->flags & XIKeyRepeat)
return;
keycode = xev->detail;
/* If the input method specified a keysym to use, use it. */
if (keysym)
{
/* If looking up the event keycode also results in the
keysym, then just use the keycode specified in the
event. */
if (LookupKeysym (xev, keycode) == keysym)
keycode = xev->detail;
else
keycode = XKeysymToKeycode (compositor.display, keysym);
/* But if no corresponding keycode could be found, use the
keycode provided in the event. */
if (!keycode)
keycode = xev->detail;
}
/* If the input method specified a keycode, use it. */
if (!keycode)
keycode = xev->detail;
if (xev->evtype == XI_KeyPress)
SendKeyboardKey (seat, seat->focus_surface,
@ -5586,11 +5550,13 @@ XLSeatExplicitlyGrabSurface (Seat *seat, Surface *surface, uint32_t serial)
/* Now, grab the keyboard. Note that we just grab the keyboard so
that keyboard focus cannot be changed, which is not very crucial,
so it is allowed to fail. */
so it is allowed to fail. The keyboard grab is not owner_events,
which is important for input method events to be filtered
correctly. */
state = XIGrabDevice (compositor.display, seat->master_keyboard,
window, time, None, XIGrabModeAsync,
XIGrabModeAsync, True, &mask);
XIGrabModeAsync, False, &mask);
/* Cancel any external grab that might be applied if the keyboard
grab succeeded. */

View file

@ -56,7 +56,7 @@ struct _PropertyAtom
struct _SelectionOwnerInfo
{
/* When this selection was last owned. */
Time time;
Timestamp time;
/* The targets of this atom. */
Atom *targets;
@ -1421,7 +1421,10 @@ HandleSelectionRequest (XEvent *event)
if (!info
|| !info->get_transfer_function
|| event->xselectionrequest.time < info->time
/* The ICCCM prohibits clients from using CurrentTime, but some
do anyway. */
|| (event->xselectionrequest.time != CurrentTime
&& TimeIs (event->xselectionrequest.time, Earlier, info->time))
|| !CanConvertTarget (info, event->xselectionrequest.target))
{
DebugPrint ("Couldn't convert selection due to simple reason\n");
@ -1805,7 +1808,7 @@ CompleteDelayedTransfer (ReadTransfer *transfer)
}
Bool
OwnSelection (Time time, Atom selection,
OwnSelection (Timestamp time, Atom selection,
GetDataFunc (*hook) (WriteTransfer *transfer,
Atom, Atom *),
Atom *targets, int ntargets)
@ -1815,15 +1818,13 @@ OwnSelection (Time time, Atom selection,
info = XLLookUpAssoc (selection_owner_info, selection);
if (time == CurrentTime)
time = XLGetServerTimeRoundtrip ();
if (info && time < info->time)
if (info && TimestampIs (time, Earlier, info->time))
/* The timestamp is out of date. */
return False;
XSetSelectionOwner (compositor.display, selection,
selection_transfer_window, time);
selection_transfer_window,
time.milliseconds);
/* Check if selection ownership was actually set. */
owner = XGetSelectionOwner (compositor.display, selection);
@ -1863,7 +1864,7 @@ DisownSelection (Atom selection)
if (info && info->get_transfer_function)
{
XSetSelectionOwner (compositor.display, selection,
None, info->time);
None, info->time.milliseconds);
/* Also free info->targets. */
XLFree (info->targets);

11
test.c
View file

@ -552,11 +552,22 @@ GetTestSeat (struct wl_client *client, struct wl_resource *resource,
XLGetTestSeat (client, resource, id);
}
static void
GetSerial (struct wl_client *client, struct wl_resource *resource)
{
uint32_t serial;
/* Send the display's next serial to the client. */
serial = wl_display_next_serial (compositor.wl_display);
test_manager_send_serial (resource, serial);
}
static const struct test_manager_interface test_manager_impl =
{
.get_test_surface = GetTestSurface,
.get_scale_lock = GetScaleLock,
.get_test_seat = GetTestSeat,
.get_serial = GetSerial,
};

View file

@ -1102,6 +1102,33 @@ GetDeviceController (struct wl_client *client, struct wl_resource *resource,
HandleTestDeviceControllerDestroy);
}
static void
SetLastUserTime (struct wl_client *client, struct wl_resource *resource,
uint32_t months, uint32_t milliseconds)
{
Timestamp timestamp;
TestSeatController *controller;
timestamp.months = months;
timestamp.milliseconds = milliseconds;
controller = wl_resource_get_user_data (resource);
if (TimestampIs (timestamp, Earlier,
controller->seat->last_user_time))
{
wl_resource_post_error (resource, TEST_MANAGER_ERROR_INVALID_USER_TIME,
"the specified user time (%"PRIu32":%"PRIu32
") lies in the past. the current time is %u:%u",
months, milliseconds,
controller->seat->last_user_time.months,
controller->seat->last_user_time.milliseconds);
return;
}
controller->seat->last_user_time.months = months;
controller->seat->last_user_time.milliseconds = milliseconds;
}
static const struct test_seat_controller_interface seat_controller_impl =
{
.destroy = DestroySeatController,
@ -1115,6 +1142,7 @@ static const struct test_seat_controller_interface seat_controller_impl =
.dispatch_XI_ButtonPress = DispatchXIButtonPress,
.dispatch_XI_ButtonRelease = DispatchXIButtonRelease,
.get_device_controller = GetDeviceController,
.set_last_user_time = SetLastUserTime,
};
static void

View file

@ -3,7 +3,7 @@
12TO11ROOT = ..
DEPLIBS = $(DEPXLIB)
SYS_LIBRARIES = MathLibrary
LOCAL_LIBRARIES = $(WAYLAND_CLIENT) $(XLIB) $(PNG) $(GBM) $(DRM)
LOCAL_LIBRARIES = $(WAYLAND_CLIENT) $(XLIB) $(PNG)
COMMONOBJS = test_harness.o
COMMONSRCS = test_harness.c
HEADER = test_harness.h
@ -46,7 +46,11 @@ ScannerTarget(linux-dmabuf-unstable-v1)
OBJS8 = $(COMMONSRCS) seat_test.o
SRCS9 = $(COMMONSRCS) dmabuf_test.c
OBJS9 = $(COMMONSRCS) dmabuf_test.o
PROGRAMS = imgview simple_test damage_test transform_test viewporter_test subsurface_test scale_test seat_test dmabuf_test
SRCS10 = $(COMMONSRCS) select_test.c
OBJS10 = $(COMMONSRCS) select_test.o
SRCS11 = select_helper.c
OBJS11 = select_helper.o
PROGRAMS = imgview simple_test damage_test transform_test viewporter_test subsurface_test scale_test seat_test dmabuf_test select_test select_helper
/* Make all objects depend on HEADER. */
$(OBJS1): $(HEADER)
@ -62,10 +66,13 @@ NormalProgramTarget(viewporter_test,$(OBJS5),NullParameter,$(LOCAL_LIBRARIES),Nu
NormalProgramTarget(subsurface_test,$(OBJS6),NullParameter,$(LOCAL_LIBRARIES),NullParameter)
NormalProgramTarget(scale_test,$(OBJS7),NullParameter,$(LOCAL_LIBRARIES),NullParameter)
NormalProgramTarget(seat_test,$(OBJS8),NullParameter,$(LOCAL_LIBRARIES),NullParameter)
NormalProgramTarget(dmabuf_test,$(OBJS9),NullParameter,$(LOCAL_LIBRARIES),NullParameter);
NormalProgramTarget(dmabuf_test,$(OBJS9),NullParameter,$(LOCAL_LIBRARIES) $(GBM) $(DRM),NullParameter);
NormalProgramTarget(select_test,$(OBJS10),NullParameter,$(LOCAL_LIBRARIES) ThreadsLibraries,NullParameter);
NormalProgramTarget(select_helper,$(OBJS11),NullParameter,$(XLIB),NullParameter);
DependTarget3($(SRCS1),$(SRCS2),$(SRCS3))
DependTarget3($(SRCS4),$(SRCS5),$(SRCS6))
DependTarget3($(SRCS7),$(SRCS8),NullParameter)
DependTarget3($(SRCS7),$(SRCS8),$(SRCS9))
DependTarget3($(SRCS10),$(SRCS11),NullParameter)
all:: $(PROGRAMS)

View file

@ -20,5 +20,7 @@ would be ``integration tests''.
Most likely, you do not want to run these tests manually, as the
`run_tests.sh' script does all the setup for you.
Please note that the EGL renderer currently does not pass the
viewporter test, which is expected behavior.
Please note that the EGL renderer currently does not pass some
graphics tests, which is expected behavior, and that `select_test'
must be run with no clipboard manager (or any other clients, for that
matter) running.

View file

@ -45,4 +45,6 @@ do
fi
done
# TODO: figure out how to run some tests under Xvfb.
popd

View file

@ -9,6 +9,8 @@ subsurface_test
scale_test
seat_test
dmabuf_test
select_test
select_helper
imgview
reject.dump
Makefile

View file

@ -61,9 +61,20 @@ handle_test_manager_display_string (void *data, struct test_manager *manager,
&display->num_pixmap_formats);
}
static void
handle_test_manager_serial (void *data, struct test_manager *manager,
uint32_t serial)
{
struct test_display *display;
display = data;
display->serial = serial;
}
static const struct test_manager_listener test_manager_listener =
{
handle_test_manager_display_string,
handle_test_manager_serial,
};
static bool
@ -220,6 +231,7 @@ open_test_display (struct test_interface *interfaces, int num_interfaces)
if (!display->scale_lock)
goto error_4;
display->seat = NULL;
return display;
error_4:
@ -901,3 +913,12 @@ test_complete (void)
test_log ("test ran successfully");
exit_with_code (0);
}
uint32_t
test_get_serial (struct test_display *display)
{
test_manager_get_serial (display->test_manager);
wl_display_roundtrip (display->display);
return display->serial;
}

View file

@ -87,6 +87,9 @@ struct test_display
/* The number of such interfaces. */
int num_test_interfaces;
/* Internal field used by test_get_serial. */
uint32_t serial;
};
struct test_interface
@ -152,6 +155,7 @@ extern void test_init (void);
extern void test_complete (void) __attribute__ ((noreturn));
extern void test_set_scale (struct test_display *, int);
extern void test_init_seat (struct test_display *);
extern uint32_t test_get_serial (struct test_display *);
#define ARRAYELTS(arr) (sizeof (arr) / sizeof (arr)[0])

View file

@ -2488,7 +2488,8 @@ CreateIC (TextInput *input)
XLAssert (!input->xic);
DebugPrint ("creating XIC for text input %p", input);
DebugPrint ("creating XIC for text input %p and window 0x%lx", input,
window);
status_attr = NULL;
preedit_attr = NULL;
@ -3301,16 +3302,47 @@ LookupString (TextInput *input, XEvent *event, KeySym *keysym_return)
return True;
}
static KeyCode
CalculateKeycodeForEvent (XEvent *event, KeySym keysym)
{
KeySym sym_return;
unsigned int mods_return;
/* Calculate the keycode to actually send clients along with an
event, given the keysym specified by the input method. Return 0
if no special treatment is required. */
if (!keysym)
return 0;
/* If looking up the event keycode also results in the keysym,
then just use the keycode specified in the event. This is
because French keyboard layouts have multiple keycodes that
decode to the same keysym, which causes problems later on
when Wayland clients keep repeating the "a" key, as a keysym
was looked up for the key press but not for the corresponding
key release. */
if (XkbLookupKeySym (compositor.display, event->xkey.keycode,
event->xkey.state, &mods_return, &sym_return)
&& keysym == sym_return)
return 0;
/* Otherwise, convert the keysym to a keycode and use that. */
return XLKeysymToKeycode (keysym, event);
}
static Bool
FilterInputCallback (Seat *seat, Surface *surface, void *event,
KeySym *keysym)
KeyCode *keycode)
{
XIDeviceEvent *xev;
XEvent xkey;
TextInputClientInfo *info;
TextInput *input;
KeySym keysym;
xev = event;
keysym = 0;
DebugPrint ("seat %p, surface %p, detail: %d, event: %lx",
seat, surface, xev->detail, xev->event);
@ -3342,7 +3374,19 @@ FilterInputCallback (Seat *seat, Surface *surface, void *event,
/* Otherwise, call XmbLookupString. If a keysym is
returned, return False. Otherwise, commit the string
looked up and return True. */
return LookupString (input, &xkey, keysym);
if (LookupString (input, &xkey, &keysym))
return True;
/* Look up the right keycode for the event. */
if (!keysym && xev->type == XI_KeyRelease)
*keycode = GetKeycode (&input->keysym_map, xev->detail);
else
*keycode = CalculateKeycodeForEvent (&xkey, keysym);
if (xev->type == XI_KeyRelease)
/* Remove the keycode from the keycode-keysym map. */
RemoveKeysym (&input->keysym_map, xev->detail);
}
}
@ -3360,35 +3404,6 @@ static TextInputFuncs input_funcs =
.filter_input = FilterInputCallback,
};
static KeyCode
CalculateKeycodeForEvent (XEvent *event, KeySym keysym)
{
KeySym sym_return;
unsigned int mods_return;
/* Calculate the keycode to actually send clients along with an
event, given the keysym specified by the input method. Return 0
if no special treatment is required. */
if (!keysym)
return 0;
/* If looking up the event keycode also results in the keysym,
then just use the keycode specified in the event. This is
because French keyboard layouts have multiple keycodes that
decode to the same keysym, which causes problems later on
when Wayland clients keep repeating the "a" key, as a keysym
was looked up for the key press but not for the corresponding
key release. */
if (XkbLookupKeySym (compositor.display, event->xkey.keycode,
event->xkey.state, &mods_return, &sym_return)
&& keysym == sym_return)
return 0;
/* Otherwise, convert the keysym to a keycode and use that. */
return XLKeysymToKeycode (keysym, event);
}
void
XLTextInputDispatchCoreEvent (Surface *surface, XEvent *event)
{
@ -3436,8 +3451,12 @@ XLTextInputDispatchCoreEvent (Surface *surface, XEvent *event)
return;
if (info->focus_surface != surface)
/* The surface is no longer focused. */
return;
{
/* The surface is no longer focused. */
DebugPrint ("incorrect focus surface (focus surface is %p, "
"surface is %p)", info->focus_surface, surface);
return;
}
if (info)
{

17
xdata.c
View file

@ -1201,20 +1201,20 @@ static void
SelectSelectionInput (Atom selection)
{
int mask;
Time time;
/* If the selection already exists, announce it to Wayland clients
as well. Use CurrentTime (even though the ICCCM says this is a
bad idea); any XFixesSelectionNotify event that arrives later
will clear up our (incorrect) view of the selection change
time. */
as well. Use the current time. */
time = XLGetServerTimeRoundtrip ();
if (selection == CLIPBOARD
&& XGetSelectionOwner (compositor.display, CLIPBOARD) != None)
NoticeClipboardChanged (CurrentTime);
NoticeClipboardChanged (time);
if (selection == XA_PRIMARY
&& XGetSelectionOwner (compositor.display, XA_PRIMARY) != None)
NoticePrimaryChanged (CurrentTime);
NoticePrimaryChanged (time);
mask = XFixesSetSelectionOwnerNotifyMask;
mask |= XFixesSelectionWindowDestroyNotifyMask;
@ -1995,7 +1995,8 @@ XLOwnDragSelection (Time time, DataSource *source)
/* Own the selection. */
drag_data_source = source;
rc = OwnSelection (time, XdndSelection, GetDragCallback,
rc = OwnSelection (TimestampFromClientTime (time),
XdndSelection, GetDragCallback,
targets, ntargets);
XLFree (targets);
@ -2135,7 +2136,7 @@ FindTargetInArray (Atom *targets, int ntargets, Atom atom)
/* And own the selection. */ \
field = source; \
\
rc = OwnSelection (time.milliseconds, atom, callback, targets, ntargets); \
rc = OwnSelection (time, atom, callback, targets, ntargets); \
XLFree (targets); \
return rc \