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 618312 - No native gnome-shell Bluetooth applet
No native gnome-shell Bluetooth applet
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Dan Winship
gnome-shell-maint
: 634328 (view as bug list)
Depends on: 626759 633361 634781 635023
Blocks: 634328
 
 
Reported: 2010-05-10 20:41 UTC by Michael Mulqueen
Modified: 2010-12-18 18:13 UTC
See Also:
GNOME target: 3.0
GNOME version: ---


Attachments
Bluetooth status indicator (27.14 KB, patch)
2010-10-27 21:15 UTC, Giovanni Campagna
none Details | Review
Bluetooth status indicator (25.55 KB, patch)
2010-10-28 21:09 UTC, Giovanni Campagna
needs-work Details | Review
Bluetooth status indicator (24.75 KB, patch)
2010-10-31 17:34 UTC, Giovanni Campagna
none Details | Review
Bluetooth status indicator (24.76 KB, patch)
2010-11-10 17:34 UTC, Giovanni Campagna
reviewed Details | Review
Bluetooth status indicator (26.14 KB, patch)
2010-11-16 21:04 UTC, Giovanni Campagna
none Details | Review
popupMenu: add expand/align flags to menuitem layout (7.15 KB, patch)
2010-11-17 19:34 UTC, Dan Winship
reviewed Details | Review
popupMenu: minor submenu improvements (4.02 KB, patch)
2010-11-17 19:34 UTC, Dan Winship
none Details | Review
Bluetooth status indicator (26.01 KB, patch)
2010-11-21 17:13 UTC, Giovanni Campagna
reviewed Details | Review
Bluetooth status indicator (27.82 KB, patch)
2010-12-17 17:41 UTC, Giovanni Campagna
committed Details | Review
fixes (1.70 KB, patch)
2010-12-18 17:25 UTC, Dan Winship
none Details | Review

Description Michael Mulqueen 2010-05-10 20:41:53 UTC
In Gnome 2 on Ubuntu, there is a panel applet that allows easy control of bluetooth. When I use Gnome Shell, there is no easy way to perform these usual functions. I can't conveniently browse files any more. That's the main function that I use, but others probably use more.
Comment 1 Giovanni Campagna 2010-10-27 21:15:29 UTC
Created attachment 173355 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.
Comment 2 Bastien Nocera 2010-10-27 23:38:23 UTC
I'd rather the calls for bluetooth-sendto and mounting/browsing obex:///[] URIs were moved into the private library in gnome-bluetooth, and implemented there.

Same thing for the calls to "gnome-shell-bluetooth-chooser" (which contains your home install path anyway).
Comment 3 Giovanni Campagna 2010-10-28 12:29:55 UTC
(In reply to comment #2)
> I'd rather the calls for bluetooth-sendto and mounting/browsing obex:///[] URIs
> were moved into the private library in gnome-bluetooth, and implemented there.

Ok, I'll prepare a patch for that then.

> Same thing for the calls to "gnome-shell-bluetooth-chooser" (which contains
> your home install path anyway).

Ugh... only gnome-shell-bluetooth-chooser.in had to go in.

Obsoleting for the time being...
Comment 4 Giovanni Campagna 2010-10-28 21:09:05 UTC
Created attachment 173438 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.
Comment 5 Bastien Nocera 2010-10-28 21:34:19 UTC
Review of attachment 173438 [details] [review]:

Looking better, just minor niggles now.

::: js/helpers/bluetoothChooser.js
@@ +1,1 @@
+/* -*- mode: js2; js2-basic-offset: 4; indent-tabs-mode: nil -*- */

This file will obviously need to go...

::: js/ui/panel.js
@@ +34,3 @@
     'a11y': imports.ui.status.accessibility.ATIndicator,
     'volume': imports.ui.status.volume.Indicator,
+    'bluetooth': imports.ui.status.bluetooth.Indicator,

Would it just fail to build, or run if gnome-bluetooth wasn't available? gnome-bluetooth only works on Linux systems...

::: js/ui/status/bluetooth.js
@@ +26,3 @@
+}
+
+const BluetoothType = {

This is already exported by libgnome-bluetooth, why re-declare it?

@@ +155,3 @@
+            let d = devices[i];
+            let item = this._createDeviceItem(d);
+            this.menu.addMenuItem(item, 6 + this._deviceItems.length);

We don't create menu items for devices which wouldn't have a sub-menu, so you need to filter out all the "!device.can_connect" devices.

::: src/gnome-shell-bluetooth-chooser.in
@@ +17,3 @@
+export GSETTINGS_SCHEMA_DIR="$schemaDir"
+
+@GJS_CONSOLE@ -c "const BluetoothChooserHelper = imports.helpers.bluetoothChooser;

Still don't see why that file is needed.
Comment 6 Giovanni Campagna 2010-10-29 12:53:52 UTC
(In reply to comment #5)
> Review of attachment 173438 [details] [review]:
> 
> Looking better, just minor niggles now.
> 
> ::: js/ui/panel.js
> @@ +34,3 @@
>      'a11y': imports.ui.status.accessibility.ATIndicator,
>      'volume': imports.ui.status.volume.Indicator,
> +    'bluetooth': imports.ui.status.bluetooth.Indicator,
> 
> Would it just fail to build, or run if gnome-bluetooth wasn't available?
> gnome-bluetooth only works on Linux systems...

Well, since libgnome-bluetooth-applet is a private library, libgnome-shell is linked explicitly against it, so it would fail to link if not installed.
I should add a configure check and possibly make it optional.

> 
> ::: js/ui/status/bluetooth.js
> @@ +26,3 @@
> +}
> +
> +const BluetoothType = {
> 
> This is already exported by libgnome-bluetooth, why re-declare it?

Because I don't want to load GnomeBluetooth, that would be another typelib to load and a lot more types to resolve. Having it inline in JS is a lot faster.

> 
> @@ +155,3 @@
> +            let d = devices[i];
> +            let item = this._createDeviceItem(d);
> +            this.menu.addMenuItem(item, 6 + this._deviceItems.length);
> 
> We don't create menu items for devices which wouldn't have a sub-menu, so you
> need to filter out all the "!device.can_connect" devices.

Ok.
(assuming that a device with .capabilities != None but .can_connect == false is still shown)

> ::: src/gnome-shell-bluetooth-chooser.in
> @@ +17,3 @@
> +export GSETTINGS_SCHEMA_DIR="$schemaDir"
> +
> +@GJS_CONSOLE@ -c "const BluetoothChooserHelper =
> imports.helpers.bluetoothChooser;
> 
> Still don't see why that file is needed.

Like in bug 633361, because you cannot have dialogs in the Shell process.
The only alternative is to have a bluetooth-chooser or bluetooth-browse in C in gnome-bluetooth, like we have bluetooth-sendto.
(You cannot use system dialogs, as BluetoothChooser is Gtk based and would be horrible in a St dialog)
Comment 7 Bastien Nocera 2010-10-29 13:15:01 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Review of attachment 173438 [details] [review] [details]:
> > 
> > Looking better, just minor niggles now.
> > 
> > ::: js/ui/panel.js
> > @@ +34,3 @@
> >      'a11y': imports.ui.status.accessibility.ATIndicator,
> >      'volume': imports.ui.status.volume.Indicator,
> > +    'bluetooth': imports.ui.status.bluetooth.Indicator,
> > 
> > Would it just fail to build, or run if gnome-bluetooth wasn't available?
> > gnome-bluetooth only works on Linux systems...
> 
> Well, since libgnome-bluetooth-applet is a private library, libgnome-shell is
> linked explicitly against it, so it would fail to link if not installed.
> I should add a configure check and possibly make it optional.

Agreed. Would be nicer as a run-time detection (so gnome-bluetooth can be installed after gnome-shell).

> > ::: js/ui/status/bluetooth.js
> > @@ +26,3 @@
> > +}
> > +
> > +const BluetoothType = {
> > 
> > This is already exported by libgnome-bluetooth, why re-declare it?
> 
> Because I don't want to load GnomeBluetooth, that would be another typelib to
> load and a lot more types to resolve. Having it inline in JS is a lot faster.

Right. But the applet library already knows internall about those. We could add bluetooth-enums.h to the introspection data (gnome-bluetooth-enum-types.[ch] in lib/ already do that).

> > @@ +155,3 @@
> > +            let d = devices[i];
> > +            let item = this._createDeviceItem(d);
> > +            this.menu.addMenuItem(item, 6 + this._deviceItems.length);
> > 
> > We don't create menu items for devices which wouldn't have a sub-menu, so you
> > need to filter out all the "!device.can_connect" devices.
> 
> Ok.
> (assuming that a device with .capabilities != None but .can_connect == false is
> still shown)

This is the code in the legacy applet:
static gboolean
device_has_submenu (BluetoothSimpleDevice *dev)
{
        if (dev->can_connect != FALSE)
                return TRUE;
        if (dev->capabilities != BLUETOOTH_CAPABILITIES_NONE)
                return TRUE;
        return FALSE;
}

> > ::: src/gnome-shell-bluetooth-chooser.in
> > @@ +17,3 @@
> > +export GSETTINGS_SCHEMA_DIR="$schemaDir"
> > +
> > +@GJS_CONSOLE@ -c "const BluetoothChooserHelper =
> > imports.helpers.bluetoothChooser;
> > 
> > Still don't see why that file is needed.
> 
> Like in bug 633361, because you cannot have dialogs in the Shell process.
> The only alternative is to have a bluetooth-chooser or bluetooth-browse in C in
> gnome-bluetooth, like we have bluetooth-sendto.
> (You cannot use system dialogs, as BluetoothChooser is Gtk based and would be
> horrible in a St dialog)

Then it looks like we might need to reimplement a chooser in St. Does St have a treeview widget that would use GtkTreeModel? If so, reimplementing one should be pretty straight forward.
Comment 8 Giovanni Campagna 2010-10-29 13:30:48 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Review of attachment 173438 [details] [review] [details] [details]:
> > > 
> > > Looking better, just minor niggles now.
> > > 
> > > ::: js/ui/panel.js
> > > @@ +34,3 @@
> > >      'a11y': imports.ui.status.accessibility.ATIndicator,
> > >      'volume': imports.ui.status.volume.Indicator,
> > > +    'bluetooth': imports.ui.status.bluetooth.Indicator,
> > > 
> > > Would it just fail to build, or run if gnome-bluetooth wasn't available?
> > > gnome-bluetooth only works on Linux systems...
> > 
> > Well, since libgnome-bluetooth-applet is a private library, libgnome-shell is
> > linked explicitly against it, so it would fail to link if not installed.
> > I should add a configure check and possibly make it optional.
> 
> Agreed. Would be nicer as a run-time detection (so gnome-bluetooth can be
> installed after gnome-shell).

Can't do. We need to have the module loaded when opening the typelib, and dlopen fails (not in system search directory), so we need to set RPATH with libtool.

> 
> > > ::: js/ui/status/bluetooth.js
> > > @@ +26,3 @@
> > > +}
> > > +
> > > +const BluetoothType = {
> > > 
> > > This is already exported by libgnome-bluetooth, why re-declare it?
> > 
> > Because I don't want to load GnomeBluetooth, that would be another typelib to
> > load and a lot more types to resolve. Having it inline in JS is a lot faster.
> 
> Right. But the applet library already knows internall about those. We could add
> bluetooth-enums.h to the introspection data (gnome-bluetooth-enum-types.[ch] in
> lib/ already do that).

Ok.

> 
> > > @@ +155,3 @@
> > > +            let d = devices[i];
> > > +            let item = this._createDeviceItem(d);
> > > +            this.menu.addMenuItem(item, 6 + this._deviceItems.length);
> > > 
> > > We don't create menu items for devices which wouldn't have a sub-menu, so you
> > > need to filter out all the "!device.can_connect" devices.
> > 
> > Ok.
> > (assuming that a device with .capabilities != None but .can_connect == false is
> > still shown)
> 
> This is the code in the legacy applet:
> static gboolean
> device_has_submenu (BluetoothSimpleDevice *dev)
> {
>         if (dev->can_connect != FALSE)
>                 return TRUE;
>         if (dev->capabilities != BLUETOOTH_CAPABILITIES_NONE)
>                 return TRUE;
>         return FALSE;
> }
> 
> > > ::: src/gnome-shell-bluetooth-chooser.in
> > > @@ +17,3 @@
> > > +export GSETTINGS_SCHEMA_DIR="$schemaDir"
> > > +
> > > +@GJS_CONSOLE@ -c "const BluetoothChooserHelper =
> > > imports.helpers.bluetoothChooser;
> > > 
> > > Still don't see why that file is needed.
> > 
> > Like in bug 633361, because you cannot have dialogs in the Shell process.
> > The only alternative is to have a bluetooth-chooser or bluetooth-browse in C in
> > gnome-bluetooth, like we have bluetooth-sendto.
> > (You cannot use system dialogs, as BluetoothChooser is Gtk based and would be
> > horrible in a St dialog)
> 
> Then it looks like we might need to reimplement a chooser in St. Does St have a
> treeview widget that would use GtkTreeModel? If so, reimplementing one should
> be pretty straight forward.

There is no TreeView in St, and implementing one will be a lot of work. I'd stick with the helper hack for now.
Comment 9 Bastien Nocera 2010-10-29 13:51:08 UTC
(In reply to comment #8)
> There is no TreeView in St, and implementing one will be a lot of work. I'd
> stick with the helper hack for now.

Given that we only show paired devices with the obex ftp service in this list, it might make sense to remove the menu entry altogether. As long as we keep the device sub-menus.

The same functionality would be available in the device specific sub-menus.
Comment 10 Giovanni Campagna 2010-10-31 17:34:51 UTC
Created attachment 173601 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.

This mostly works, except:
- Last rework of PopupMenus regressed submenus, and now they're always on
  the right.
- Using clutter_actor_destroy for removing device submenus on update makes
  shell fail with SIGABRT (related to memory management, comes from gjs)
Comment 11 Giovanni Campagna 2010-11-10 17:34:45 UTC
Created attachment 174209 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.

Updating labels for bug 634328
Comment 12 Dan Winship 2010-11-12 17:42:58 UTC
*** Bug 634328 has been marked as a duplicate of this bug. ***
Comment 13 Dan Winship 2010-11-15 20:46:04 UTC
Comment on attachment 174209 [details] [review]
Bluetooth status indicator

>+               [if test "x$with_bluetooth" != xauto; then
>+                  AC_MSG_FAILURE(
>+                   [--with-bluetooth was given, but gnome-bluetooth is not installed])
>+               fi

The issue here is that gnome-bluetooth only builds on Linux, correct? If so, I think we should make it explicitly mandatory on Linux, and "optional" only on other platforms; we don't allow people to configure out any of the other indicators so there's no reason to make this one optional either.

Also, I don't foresee many other uses for js/misc/config, so I think it might be better to just make this a ShellGlobal property.

>-    addMenuItem: function(menuItem) {
>-        this._box.add(menuItem.actor);
>+    addMenuItem: function(menuItem, position) {
>+        if (position != undefined)
>+            this._box.insert_actor(menuItem.actor, position);
>+        else
>+            this._box.add(menuItem.actor);

if you want to split this patch out and just commit it now, go ahead.

>+    _init: function() {
>+        PanelMenu.SystemStatusButton.prototype._init.call(this, 'bluetooth-disabled-symbolic', null);

don't need to specify "-symbolic"

>+        this._killswitch.connect('toggled', Lang.bind(this, function() {
>+            let current_state = this._applet.killswitch_state;

The logic in this handler is sort of convoluted: you're responding to the user toggling the menu item, but you never actually look at what state the menu item is in, you only look at the killswitch state, and assume that it must match.

I'd check if current_state is HARD_BLOCKED, and force the toggle false in that case, but if it's not HARD_BLOCKED, then set the new killswitch_state based on the toggle state, not based on current_state.

>+        this._discoverable = new PopupMenu.PopupSwitchMenuItem(_("Visibility"), this._applet.discoverable);

this item seems to always get initialized to "off" for me

>+        this._fullMenuItems = [new PopupMenu.PopupMenuItem(_("Send file to device...")),
>+                               new PopupMenu.PopupSeparatorMenuItem(),
>+                               new PopupMenu.PopupSeparatorMenuItem(),
>+                               new PopupMenu.PopupMenuItem(_("Configure new device..."))];

need to make sure the two separators get squished into one if there's nothing between them.

(Actually, we could just do that at the PopupMenu level maybe, rather than needing to worry about it both here and in the power manager patch...)

The menu item ordering and text doesn't correspond to the mockups, even ignoring the new-style-submenus thing. (Among other things, the mockups Capitalize Each Word in menu items. This applies throughout bluetooth.js, not just here.)

>+        Mainloop.idle_add(Lang.bind(this, function() {
>+            Main.messageTray.add(this._source);
>+            return false;
>+        }));

this is because Main.messageTray doesn't exist yet I suppose? It would be better to fix that. Probably start() should make a separate panel call to initialize the status items after the big block of constructor calls.

>+        this._applet.connect('notify::show-full-menu', Lang.bind(this, this._updateFullMenu));

The daemon is not supposed to be deciding what we do or don't show.

>+        this._source = new Source();

The message tray will destroy unused sources; you want to create a source on demand, and keep reusing it until the tray destroys it, and then create a new source on demand the next time you need one.

>+        if (on) {
>+            this._discoverable.actor.show();
>+            this.setIcon('bluetooth-symbolic');
>+        } else {
>+            this._discoverable.actor.hide();
>+            this.setIcon('bluetooth-disabled-symbolic');

again, don't need "-symbolic". Also, there doesn't seem to be a symbolic version of "bluetooth", so it falls back to the fullcolor version. You want "bluetooth-active" I guess. (Or maybe the symbolic icon theme should be installing a symlink.)

>+    _updateDevices: function() {

I can't get my laptop to pair successfully with my phone (with gnome-shell / gnome-bluetooth master) so I haven't been able to test this part...

>+    notify: function(notification) {
>+        this._notifications.push(notification);
>+
>+        MessageTray.Source.prototype.notify.call(this, notification);
>+    },

you need to remove it from this._notifications when it is destroyed
Comment 14 Giovanni Campagna 2010-11-16 20:11:57 UTC
(In reply to comment #13)
> (From update of attachment 174209 [details] [review])
> >+               [if test "x$with_bluetooth" != xauto; then
> >+                  AC_MSG_FAILURE(
> >+                   [--with-bluetooth was given, but gnome-bluetooth is not installed])
> >+               fi
> 
> The issue here is that gnome-bluetooth only builds on Linux, correct? If so, I
> think we should make it explicitly mandatory on Linux, and "optional" only on
> other platforms; we don't allow people to configure out any of the other
> indicators so there's no reason to make this one optional either.
> 
> Also, I don't foresee many other uses for js/misc/config, so I think it might
> be better to just make this a ShellGlobal property.

Unfortunately, ShellGlobal is not there when importing the panel module (which imports ui.status.bluetooth, which itself imports the non existing gi.GnomeBluetooth).

> 
> >-    addMenuItem: function(menuItem) {
> >-        this._box.add(menuItem.actor);
> >+    addMenuItem: function(menuItem, position) {
> >+        if (position != undefined)
> >+            this._box.insert_actor(menuItem.actor, position);
> >+        else
> >+            this._box.add(menuItem.actor);
> 
> if you want to split this patch out and just commit it now, go ahead.

Pushed.
Comment 15 Giovanni Campagna 2010-11-16 21:04:25 UTC
Created attachment 174632 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.


I tested the agent and had to make some fixes. Unfortunately, I cannot
complete pairing (the UI appears for a moment, just to be immediately
destroyed by CancelRequest).
Comment 16 Dan Winship 2010-11-17 19:34:47 UTC
Created attachment 174710 [details] [review]
popupMenu: add expand/align flags to menuitem layout

Mixing submenu menuitems and toggle menuitems results in poor layout.
The fix is to right-align the submenu arrows. Since we already need to
right-align the battery percentages as well, add alignment support to
PopupBaseMenuItem.addActor(), and update stuff for that.
Comment 17 Dan Winship 2010-11-17 19:34:53 UTC
Created attachment 174711 [details] [review]
popupMenu: minor submenu improvements

Use a Unicode triangle character rather than a ">" for the submenu
indicator, and have the submenu appear to the left of the parent
rather than to the right, since for the status are menus, putting it
on the right would put it off screen. (This will become irrelevant
when the submenu style is redone.)

Also, fix boxpointer to not ever draw the arrow overlapping the
rounded box corners (or even immediately adjacent to them, since that
also looks bad).
Comment 18 Giovanni Campagna 2010-11-18 14:46:11 UTC
Review of attachment 174710 [details] [review]:

::: js/ui/popupMenu.js
@@ +156,3 @@
+        }
+
+        this._children.push(params);

The code uses child, but stores params.actor, which means it will break if I happen to pass a different actor.
This should not be allowed: don't make "actor" a parameter, instead add it to the params object later, just before pushing it to _children.
Comment 19 Dan Winship 2010-11-18 20:36:32 UTC
So, when I try to pair one of my computers to the other, I seem to be getting a libnotify-based notification about it, not the direct-from-javascript notification
Comment 20 Giovanni Campagna 2010-11-18 21:34:23 UTC
That's because you didn't kill bluetooth-applet.
I don't know if the icon should kill it, in GNOME 3.0 it won't be autostarted so no need to kill it.
Comment 21 Giovanni Campagna 2010-11-21 17:13:17 UTC
Created attachment 174971 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.

Finally tested, I can confirm that it works with my phone. It requires
some designer work on agent notification, though.
Comment 22 Dan Winship 2010-12-16 15:23:22 UTC
Comment on attachment 174971 [details] [review]
Bluetooth status indicator

>+  js/misc/config.js

need to add this to .gitignore too

>+        this._killswitch = new PopupMenu.PopupSwitchMenuItem(_("Bluetooth"), false);

If I turn this off, I get some warnings from inside the applet code. If I turn it back on, I get a crash.

>+        this.addBody(_("Please enter the PIN mentioned on the device."));
>+
>+        this._entry = new St.Entry();
>+        this.addActor(this._entry);

need to CSS-style this; it comes out with no border, so you can't even see that the entry is there.

also, typing Return in the entry should be equivalent to pressing OK, and typing Escape should be equivalent to Cancel.

>+                if (this._numeric)
>+                    this._applet.agent_reply_pincode(this._devicePath, parseInt(this._entry.text));
>+                else
>+                    this._applet.agent_reply_passkey(this._devicePath, this._entry.text);

I'm not sure if it's a bug here or in the applet code, but when I try to pair one of my computers to the other, it gives me a numeric PIN to type, and then when I type it, it crashes inside the applet code, in some code path where it has a number but some part of gnome-bluetooth is apparently expecting a string instead.

>+  <autotools id="gnome-control-center">

when this does get committed, make sure there are no now-redundant module additions
Comment 23 Giovanni Campagna 2010-12-17 17:41:32 UTC
Created attachment 176603 [details] [review]
Bluetooth status indicator

Introduce the new Bluetooth indicator in the System Status area. It
is written in JS with St and uses the new GnomeBluetoothApplet library.
Comment 24 Giovanni Campagna 2010-12-17 17:46:43 UTC
(In reply to comment #22)
> (From update of attachment 174971 [details] [review])
> >+        this._killswitch = new PopupMenu.PopupSwitchMenuItem(_("Bluetooth"), false);
> 
> If I turn this off, I get some warnings from inside the applet code. If I turn
> it back on, I get a crash.

bug 637476

> also, typing Return in the entry should be equivalent to pressing OK, and
> typing Escape should be equivalent to Cancel.

I'm not sure about Escape (you should be able to ignore the notification and handle it later), but done.

> >+                if (this._numeric)
> >+                    this._applet.agent_reply_pincode(this._devicePath, parseInt(this._entry.text));
> >+                else
> >+                    this._applet.agent_reply_passkey(this._devicePath, this._entry.text);
> 
> I'm not sure if it's a bug here or in the applet code, but when I try to pair
> one of my computers to the other, it gives me a numeric PIN to type, and then
> when I type it, it crashes inside the applet code, in some code path where it
> has a number but some part of gnome-bluetooth is apparently expecting a string
> instead.

There was a bug there (pincode swapped by passkey), but the expected outcome (with master gnome-bluetooth), should have been gjs complaining about wrong parameters (or automatically coercing them), not a segfault.
Anyway, fixed and tested.
Comment 25 Dan Winship 2010-12-18 17:25:34 UTC
Created attachment 176666 [details] [review]
fixes

fix a function name, remove some debugging
Comment 26 Dan Winship 2010-12-18 17:26:05 UTC
Comment on attachment 176603 [details] [review]
Bluetooth status indicator

ok to commit after squashing the attached fixes
Comment 27 Giovanni Campagna 2010-12-18 18:11:25 UTC
Comment on attachment 176603 [details] [review]
Bluetooth status indicator

Pushed with the attached fixes (plus removing the duplicate gnome-control-center module)