Comment 38 for bug 1738838

Revision history for this message
In , Mario Sánchez Prada (mariospr) wrote :

(In reply to Florian Müllner from comment #4)
> Review of attachment 361761 [details] [review]:
>
> > otherwise we risk showing inconsistent states in each place if.
>
> I'd be interested in how that sentence continues :-p

I'd be too, but I can't remember what I had in there :). I suppose it' was something along the lines of "...if the adapter is available but it's set in airplane mode".

> Joking aside, I don't think this is correct. As far as I can see,
> "nConnectedDevices == -1" means there is no Bluetooth adapter that is
> powered on - we can toggle the AirplaneMode property as much as we want
> (that's what "Turn On/Off" does after all), that's not going to change. So a
> better option may be to do
>
> this._toggleItem.actor.visible = nConnectedDevices > -1;
>
> instead ...
>
> ::: js/ui/status/bluetooth.js
> @@ +134,3 @@
>
> + // Bluetooth will be considered 'Off' if either the adapter
> + // is not available or it's set in irplane mode.
>
> *airplane

I thought the same thing, but I'm not 100% sure on whether it's possible to have a BT adapter that would be "powered on"/"available" and would still report to be in airplane mode (either soft or hard), so I thought the sane thing to do was to check both things, just in case.

What looks more clear to me, is that the same condition should be used in both places, since what you want to show in both labels depend on exactly the same thing.

I'm adding Bastien to CC, since he added the original code and might have extra info to decide what's best to do here.