After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642978 - unable to type bluetooth passcode
unable to type bluetooth passcode
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Dan Winship
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-22 17:53 UTC by sathyz
Modified: 2011-03-19 19:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
BluetoothStatus: make the pin entry focusable (1.16 KB, patch)
2011-02-22 20:10 UTC, Giovanni Campagna
needs-work Details | Review
BluetoothStatus: make the pin entry focusable and clickable (2.11 KB, patch)
2011-03-02 16:14 UTC, Giovanni Campagna
needs-work Details | Review
BluetoothStatus: don't pass NaNs to native functions (1.96 KB, patch)
2011-03-02 16:29 UTC, Giovanni Campagna
none Details | Review
BluetoothStatus: don't pass NaNs to native functions (1.57 KB, patch)
2011-03-02 16:45 UTC, Giovanni Campagna
committed Details | Review
messageTray: dismiss notifications on click, not button-release-event (7.49 KB, patch)
2011-03-03 20:17 UTC, Dan Winship
committed Details | Review

Description sathyz 2011-02-22 17:53:07 UTC
Steps:
1. Goto Bluetooth (in system status area) -> My device (in this case, Micromax Q7) -> Send files
2. Select a file.
3. Click on open
4. A progress is shown saying "Sending files via Bluetooth" and asked for a passcode in my device.
5. On entering passcode in my device, a notification with detailed information appeared asking for the passcode.
6. Clicking on the textbox(input field) for typing the passcode, closes/hides the notification.

Unable to type passcode in the given field.

Also, pressing tab switches between ok and cancel buttons, focus isn't set to the passcode field.

Thanks,
Comment 1 Giovanni Campagna 2011-02-22 20:10:41 UTC
Created attachment 181631 [details] [review]
BluetoothStatus: make the pin entry focusable

The StEntry used to get the pincode for associating with a device
needs to be focusable, or any attempt to click on it will be
captured by Notification.
Comment 2 drago01 2011-02-23 10:44:32 UTC
(In reply to comment #1)
> Created an attachment (id=181631) [details] [review]
> BluetoothStatus: make the pin entry focusable
> 
> The StEntry used to get the pincode for associating with a device
> needs to be focusable, or any attempt to click on it will be
> captured by Notification.

This is odd, can_focus just means "can be focused using the keyboard" it has nothing to do with clicks, not that it is wrong to allow that for the pin entry field but I don't understand how it fixes that bug or how it is related to clicks not getting through.

Dan?
Comment 3 Dan Winship 2011-02-23 15:05:34 UTC
agreed, this patch should not have any effect; it's "reactive" that determines whether or not an entry can accept clicks, and StEntry automatically sets that on itself
Comment 4 drago01 2011-02-23 19:43:30 UTC
Review of attachment 181631 [details] [review]:

Marking as needs-works as per comment 2 and comment 3
Comment 5 Giovanni Campagna 2011-03-02 15:55:12 UTC
(In reply to comment #3)
> agreed, this patch should not have any effect; it's "reactive" that determines
> whether or not an entry can accept clicks, and StEntry automatically sets that
> on itself

StEntry has no special handling of button-presses, according to src/st/st-entry.c, which therefore will bubble to Notification (St.Table actually) and result in "done-displaying" signals.
I need to connect to 'button-release-event' on St.Entry.
Comment 6 Giovanni Campagna 2011-03-02 16:14:36 UTC
Created attachment 182262 [details] [review]
BluetoothStatus: make the pin entry focusable and clickable

The StEntry used to get the pincode for associating with a device
needs to be focusable, and need to block the emission of
button-release-event, or any attempt to click on it will result in
closing the notification.
Also, connecting to key-press-event at the StEntry level has no
effect, because ClutterText blocks this event.
Comment 7 Giovanni Campagna 2011-03-02 16:29:42 UTC
Created attachment 182263 [details] [review]
BluetoothStatus: don't pass NaNs to native functions

GJS complains when a NaN is passed in place of an integer, and
parseInt returns that from non numeric string. Pass a sentinel
that cancels the operation in that case.

Taking advantage of this open bug to fix another issue I discovered
while testing.
Comment 8 Giovanni Campagna 2011-03-02 16:45:19 UTC
Created attachment 182272 [details] [review]
BluetoothStatus: don't pass NaNs to native functions

GJS complains when a NaN is passed in place of an integer, and
parseInt returns that from non numeric string. Pass a sentinel
that cancels the operation in that case.
Comment 9 Dan Winship 2011-03-03 20:17:32 UTC
Comment on attachment 182262 [details] [review]
BluetoothStatus: make the pin entry focusable and clickable

>needs to be focusable, and need to block the emission of
>button-release-event, or any attempt to click on it will result in
>closing the notification.

Hm. That's a bug in Notification really. Will attach a patch.

>-            } else if (key == Clutter.KEY_Escape) {
>-                this.emit('action-invoked', 'cancel');
>-                return true;

There's nothing that replaces that in the new code. Was that intentional?
Comment 10 Dan Winship 2011-03-03 20:17:48 UTC
Created attachment 182398 [details] [review]
messageTray: dismiss notifications on click, not button-release-event

Notification was connecting to button-release-event to decide when to
be dismissed, which caused problems with widgets inside the
notification that reacted to button-press-event but not
button-release-event. Fix this by wrapping the Notification's table in
an StButton and connecting to 'click'.
Comment 11 Giovanni Campagna 2011-03-07 22:55:23 UTC
(In reply to comment #9)
> (From update of attachment 182262 [details] [review])
> >-            } else if (key == Clutter.KEY_Escape) {
> >-                this.emit('action-invoked', 'cancel');
> >-                return true;
> 
> There's nothing that replaces that in the new code. Was that intentional?

Sort of. With that code working, there was no way to close the notification with the keyboard, without also cancelling the whole operation.
Comment 12 Giovanni Campagna 2011-03-09 17:43:03 UTC
Review of attachment 182398 [details] [review]:

The fix seems correct (clicked is not emitted if StButton does not first get button-press-event), but it seems more a hack than a real solution.
I saw that TelepathyClient gets away from this by adding the entry to the action area, which bluetooth cannot do (it needs buttons).
I still think that blocking button-release-event is the easiest way to fix this. Another possibility would be to add a flag to addActor that ignores clicks on it.
Comment 13 Giovanni Campagna 2011-03-09 17:43:07 UTC
Review of attachment 182398 [details] [review]:

The fix seems correct (clicked is not emitted if StButton does not first get button-press-event), but it seems more a hack than a real solution.
I saw that TelepathyClient gets away from this by adding the entry to the action area, which bluetooth cannot do (it needs buttons).
I still think that blocking button-release-event is the easiest way to fix this. Another possibility would be to add a flag to addActor that ignores clicks on it.
Comment 14 Giovanni Campagna 2011-03-13 16:09:45 UTC
Comment on attachment 182272 [details] [review]
BluetoothStatus: don't pass NaNs to native functions

Attachment 182272 [details] pushed as 25015c9 - BluetoothStatus: don't pass NaNs to native functions
Comment 15 Dan Winship 2011-03-16 20:12:59 UTC
Comment on attachment 182398 [details] [review]
messageTray: dismiss notifications on click, not button-release-event

It's not a hack; destroying on click is the behavior we wanted; the only
reason we were doing destroy on button-release instead was because it was
easier. But now doing it the easy way is messing up something else, so we
should fix it.

Attachment 182398 [details] pushed as 06d2c0a - messageTray: dismiss notifications on click, not button-release-event
Comment 16 Giovanni Campagna 2011-03-19 19:44:41 UTC
This is now fixed, I suppose.