Closed Bug 815924 Opened 8 years ago Closed 8 years ago

Wifi hotspot (tethering / internet sharing) should be disabled if data connection is not enabled

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect, P4)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:-, blocking-basecamp:-, firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

VERIFIED FIXED
B2G C4 (2jan on)
blocking-b2g -
blocking-basecamp -
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: cjones, Assigned: vchang)

References

Details

(Keywords: polish, Whiteboard: interaction [UX-P?])

Attachments

(1 file, 2 obsolete files)

STR
 (1) Make sure wifi and cellular data are disabled
 (2) Open settings app
 (3) Try to enable wifi hotspot: checkbox is there and available

What happens is that the checkbox looks like it enables, but then it immediately disables.  It looks like there's something wrong, so I try to enable again, but then the checkbox disables.

Major usability issue.

Not sure whether the backend or frontend is at fault here.
We are not allowed to enable Wifi tethering when cellular data is disabled. The backend detects this and turn off Wifi tethering automatically. That's why you see the checkbox switch off automatically. Not sure if it makes sense ?
It should probably grey (disable) the checkbox?
(In reply to Vincent Chang[:vchang] from comment #1)
> We are not allowed to enable Wifi tethering when cellular data is disabled.
> The backend detects this and turn off Wifi tethering automatically. That's
> why you see the checkbox switch off automatically. Not sure if it makes
> sense ?

That makes sense, but gaia shouldn't allow me try to enable wifi tethering to begin with in that case :).

dhylands's suggestion seems fine to me.
Larisa, is there anything new that needs to be specified here or is the settings app not implementing the spec correctly?
Flags: needinfo?(lco)
(In reply to Chris Jones [:cjones] [:warhammer] from comment #4)
> Larisa, is there anything new that needs to be specified here or is the
> settings app not implementing the spec correctly?

Agree with dhylands. The Wi-fi hotspot text and switch should be greyed out. Also grey out the descriptive text below that says "Allow other devices to share..."
Flags: needinfo?(lco)
Not a blocker, but marking as a P4 since we'd accept a low risk uplift. This work should not be prioritized over b-b+ bugs.
blocking-basecamp: ? → -
Priority: -- → P4
OS: All → Gonk (Firefox OS)
Hardware: All → ARM
Renominate, given the fact that this bug causes confusion like bug 816411 comment 1.
blocking-basecamp: - → ?
OS: Gonk (Firefox OS) → All
Hardware: ARM → All
OS: All → Gonk (Firefox OS)
Hardware: All → ARM
We would take a patch but not a blocker.
blocking-basecamp: ? → -
Priority: -- → P4
I am seriously disagree that this is a non-blocker. Ping the reporter and UX.

Also, David, you did mark bug 816411 as a blocker ... what was the baseline?
Flags: needinfo?(lco)
Flags: needinfo?(jones.chris.g)
Flags: needinfo?(dscravaglieri)
IMHO this bug makes wifi hotspot feel like a broken feature.
Flags: needinfo?(jones.chris.g)
Trying to get ppl's attention again ... I've dup a blocking+ bug to this bug so this one should be blocking.

If people doing triage fail to understand the concept, maybe I should just reopen that one and mark this one as the dup of that?
blocking-basecamp: - → ?
Keywords: late-l10n
Summary: Trying to enable wifi hotspot when no data connection is available results in checkbox "fighting" with user → Wifi hotspot should be disabled (with a warning label) if data connection is not established
No warning label according to comment 5.
Keywords: late-l10n
Summary: Wifi hotspot should be disabled (with a warning label) if data connection is not established → Wifi hotspot should be disabled if data connection is not established
Triage: We have to be very strict on what we block on now. Although this isn't great UX, we probably wouldn't block the release if this was the last bug.

blocking- but that doesn't mean a patch can't land, just that blocking+ bugs should be prioritised over this one.
blocking-basecamp: ? → -
Keywords: polish
Whiteboard: interaction
I'm absolutely not sure why it is not allowed to have internet sharing enabled while you don't have a mobile data connection. Let me give an example which I hit today a dozen of times... I have been connected via mobile data while traveling in a train. I have enabled tethering to be able to access internet on my MB. Over and over again I lost the connection for a moment and tethering has been turned off. As result I had to wait until mobile data was available again. Then I can turn tethering on again. Some minutes later the game started from the beginning. I'm sure you can understand now why I call this unacceptable. I haven't seen such a behavior by any other phone yet. All of them kept tethering enabled whether I was connected via mobile data or not.

Please reconsider your decision here because I don't think you will make users happy with that behavior.
Summary: Wifi hotspot should be disabled if data connection is not established → Wifi hotspot (tethering / internet sharing) should be disabled if data connection is not established
The problem here is that we need mobile data connection's interface name and dns server address.
Maybe we can specify the default value in device/qcom/unagi/full_xxxx.mk with 
ro.moz.connection.mobile.interface = rmnet0 
ro.moz.connection.mobile.dns1 = "8.8.8.8"
ro.moz.connection.mobile.dns2 = "8.8.4.4" 

We can update these information once the mobile data connection is established. 

Hi philikon, may I have your comment here ?
Flags: needinfo?(philipp)
As much as I'd like great UX too, I'm a bit doubtful that our first users are going to want to create a mobile hotspot on their expensive data connection anyway. So if I have to compare this to all the other blockers, I agree that it should be a blocking- (hey, unless it's literally a 5 minute fix and someone volunteers!)

Also, I guess I misunderstood part of this bug in the beginning when I said to grey out the text below the hotspot switch "Allow other devices to share my phone's internet connection...". 

Would it be too late to change the message to say "Switch on the phone's data connection to use it as a Wi-Fi hotspot" when the switch is disabled instead?
Flags: needinfo?(lco)
Depends on: 823813
Attached patch Possible fix Patch v1.0 (obsolete) — Splinter Review
Assignee: nobody → vchang
Attachment #694790 - Flags: feedback?(philipp)
Could someone please give feedback on my comment 15? Shall I file a new bug for it for post basecamp?
Henrik, this is not about disabling the hotspot when your data connection goes away temporarily. If this is happening, I would consider this a bug that should be filed and fixed. This is about not letting the user enable tethering when there's cellular data is disabled by the user in the first place.

This, btw, seems perfectly fixable in Gaia, since the Settings app knows whether or not cellular data is enabled or not.
Flags: needinfo?(philipp)
Comment on attachment 694790 [details] [diff] [review]
Possible fix Patch v1.0

Review of attachment 694790 [details] [diff] [review]:
-----------------------------------------------------------------

I fail to see how this patch addresses the problem at hand.

::: dom/system/gonk/NetworkManager.js
@@ +714,4 @@
>      if (!mobile) {
> +      try {
> +        this._tetheringInterface[TETHERING_TYPE_WIFI].externalInterface =
> +          Services.prefs.getCharPref("ril.data.interface");

Where is this pref coming from, who sets it?
Attachment #694790 - Flags: feedback?(philipp)
(In reply to Philipp von Weitershausen [:philikon] from comment #20)
> that should be filed and fixed. This is about not letting the user enable
> tethering when there's cellular data is disabled by the user in the first
> place.

Thanks. I will file a new bug for. For now adjusting the summary of this bug.
Summary: Wifi hotspot (tethering / internet sharing) should be disabled if data connection is not established → Wifi hotspot (tethering / internet sharing) should be disabled if data connection is not enabled
> Review of attachment 694790 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I fail to see how this patch addresses the problem at hand.

I tried to remove the dependency between wifi hotspot and mobile data connection in this patch. So that we can turn on hotspot even if mobile data connection is disabled. Actually, we need interface name and dns server's ip address. They can be obtained when turning on mobile data connection(from NetworkManager and net.rmnet0.dns1/net.rmnet0.dns2 property).    

> ::: dom/system/gonk/NetworkManager.js
> @@ +714,4 @@
> >      if (!mobile) {
> > +      try {
> > +        this._tetheringInterface[TETHERING_TYPE_WIFI].externalInterface =
> > +          Services.prefs.getCharPref("ril.data.interface");
> 
> Where is this pref coming from, who sets it?

We can define it in b2g/app/b2g.js or gaia/profile/user.js, or we can define them in device/qcom/unagi/full_xxxx.mk with following entries, and reading them with  libcutils.property_get() function.  
ro.moz.connection.mobile.interface = rmnet0 
ro.moz.connection.mobile.dns1 = "8.8.8.8"
ro.moz.connection.mobile.dns2 = "8.8.4.4"
(In reply to Henrik Skupin (:whimboo) [away 12/21 - 01/01] from comment #15)
> I'm absolutely not sure why it is not allowed to have internet sharing
> enabled while you don't have a mobile data connection. Let me give an
> example which I hit today a dozen of times... I have been connected via
> mobile data while traveling in a train. I have enabled tethering to be able
> to access internet on my MB. Over and over again I lost the connection for a
> moment and tethering has been turned off. As result I had to wait until
> mobile data was available again. Then I can turn tethering on again. Some
> minutes later the game started from the beginning. I'm sure you can
> understand now why I call this unacceptable. I haven't seen such a behavior
> by any other phone yet. All of them kept tethering enabled whether I was
> connected via mobile data or not.
> 
> Please reconsider your decision here because I don't think you will make
> users happy with that behavior.

I totally agree with this. Not only because of the described usecase that I met twice on a daily basis, but because i've checked on my Android device (HTC Desire Z, Android 2.3.3) and it allows to have WiFi Hotspot while no data is available. Plus, the USB sharing feature on Firefox OS also works without data available.
(In reply to Alexandre LISSY :gerard-majax from comment #24)
> I totally agree with this. Not only because of the described usecase that I
> met twice on a daily basis, but because i've checked on my Android device
> (HTC Desire Z, Android 2.3.3) and it allows to have WiFi Hotspot while no
> data is available. Plus, the USB sharing feature on Firefox OS also works
> without data available.

I don't understand this. First, I don't think this would make users not happy. Second, I don't see any useful actions by connecting to another device' wifi without 3G connection or internet tethering without 3G/Wifi connection.

Last but not the least, why would you care if HTC is doing so or not if this is a right improvement to do.

If there is some current ways of good practices by doing so, come up with some examples to persuade people. (HTC phone example is not a good example till you find out there is something meaningful behind this action)
QA Contact: wachen
(In reply to Walter Chen from comment #25)
> (In reply to Alexandre LISSY :gerard-majax from comment #24)
> > I totally agree with this. Not only because of the described usecase that I
> > met twice on a daily basis, but because i've checked on my Android device
> > (HTC Desire Z, Android 2.3.3) and it allows to have WiFi Hotspot while no
> > data is available. Plus, the USB sharing feature on Firefox OS also works
> > without data available.
> 
> I don't understand this. First, I don't think this would make users not
> happy. Second, I don't see any useful actions by connecting to another
> device' wifi without 3G connection or internet tethering without 3G/Wifi
> connection.
> 
> Last but not the least, why would you care if HTC is doing so or not if this
> is a right improvement to do.
> 
> If there is some current ways of good practices by doing so, come up with
> some examples to persuade people. (HTC phone example is not a good example
> till you find out there is something meaningful behind this action)

I'm not saying "copy HTC behavior", I'm saying "their behavior is different, it's a widespread system, maybe it has some sense ?". And I think it has: use the phone as an access point. Students of mine were using this to play games during my lectures. And as I already said, the behavior is not consistent between WiFi sharing and USB sharing: one does not get enabled when there is no data connection available while the other can be enabled.
Hi Philipp, 

I am wondering if you can help to review this patch based on comment 23 ? 

Many thinks, 
Vincent
Flags: needinfo?(philipp)
(In reply to Henrik Skupin (:whimboo) from comment #22)
> (In reply to Philipp von Weitershausen [:philikon] from comment #20)
> > that should be filed and fixed. This is about not letting the user enable
> > tethering when there's cellular data is disabled by the user in the first
> > place.
> 
> Thanks. I will file a new bug for. For now adjusting the summary of this bug.

Did you file a new bug? What bug nr?

I have the same problem with Wi-Fi hotspot turned off some time after enabling it.
I see this in the log:
02-04 15:33:10.160: I/Gecko(105): -*- WifiWorker component: Enable Wifi tethering result: mobile interface is not registered
So I guess I'm encountering this bug, but I find it weird.
I'm registered to a mobile provider and I've checked the Data connection and Data roaming option. What is going wrong here.
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #28)
> > Thanks. I will file a new bug for. For now adjusting the summary of this bug.
> 
> Did you file a new bug? What bug nr?

Looks like I missed that. :( Can you please file it? You seem to have more information available.
I'm afraid I know less than you.
I haven't been able to get this feature working on my Otoro device at all. I tried with 2 different sim cards and I made sure the mobile network was working.
Ok, I filed bug 839076 for the issue that wifi hotspot feature isn't working at all for me.
(In reply to Vincent Chang[:vchang] from comment #23)
> > Review of attachment 694790 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I fail to see how this patch addresses the problem at hand.
> 
> I tried to remove the dependency between wifi hotspot and mobile data
> connection in this patch. So that we can turn on hotspot even if mobile data
> connection is disabled. Actually, we need interface name and dns server's ip
> address. They can be obtained when turning on mobile data connection(from
> NetworkManager and net.rmnet0.dns1/net.rmnet0.dns2 property).    
> 
> > ::: dom/system/gonk/NetworkManager.js
> > @@ +714,4 @@
> > >      if (!mobile) {
> > > +      try {
> > > +        this._tetheringInterface[TETHERING_TYPE_WIFI].externalInterface =
> > > +          Services.prefs.getCharPref("ril.data.interface");
> > 
> > Where is this pref coming from, who sets it?
> 
> We can define it in b2g/app/b2g.js or gaia/profile/user.js, or we can define
> them in device/qcom/unagi/full_xxxx.mk with following entries, and reading
> them with  libcutils.property_get() function.  
> ro.moz.connection.mobile.interface = rmnet0 
> ro.moz.connection.mobile.dns1 = "8.8.8.8"
> ro.moz.connection.mobile.dns2 = "8.8.4.4"

Hi Vincent,

1. It's better to put dns1/dns2 and externalInterface values at same location for consistency, instead of separating them in pref and settings.  How do you think?
2. Is it mandatory to have pre-configured dns1/dns2 in settings?  If missing them could cause issues, we should have some fail safe implementation when they are not available.
3. If A-GPS/MMS use different APN name, it's possible that data conection for Internet moved from rmnet0 to rmnet1.  It should be considered in follow up bug.
Pretty silly bug that will frustrate users, would like to see this fixed asap.
blocking-b2g: --- → leo?
Whiteboard: interaction → interaction [UX-P?]
(In reply to Chris Jones [:cjones] [:warhammer] from comment #33)
> Pretty silly bug that will frustrate users, would like to see this fixed
> asap.

I am going to post a new patch soon to address comment 32.
This patch addresses item 1 and item 2 in comment 32. All the required parameters to enable wifi/usb tethering will be assigned the default value before read them from settings db. So we can enable wifi/usb tethering even if  3G connection is not established.

We should file a follow-up to address item 3 in comment 32, and update parameters such as dns and interface name when mobile connection status is changed.
Attachment #719372 - Flags: review?(swu)
Attachment #719372 - Flags: review?(mrbkap)
blocking-b2g: leo? → -
tracking-b2g18: --- → +
Comment on attachment 719372 [details] [diff] [review]
Patch v1.1 use default value when mobile connection is down.

Review of attachment 719372 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/NetworkManager.js
@@ +160,1 @@
>    let settingsLock = gSettingsService.createLock();

Nit, please add a blank line above the declaration of settingsLock.
Attachment #719372 - Flags: review?(mrbkap) → review+
Attachment #719372 - Flags: review?(swu) → review+
Comment on attachment 719372 [details] [diff] [review]
Patch v1.1 use default value when mobile connection is down.

Review of attachment 719372 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/NetworkManager.js
@@ +600,1 @@
>          debug("'" + aName + "'" + " is now " + this.tetheringSettings[aName]);

Putting this outside would display even when value not changed, just to make sure you did this intentionally.
Fixed the nit.
Attachment #694790 - Attachment is obsolete: true
Attachment #719372 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/f9b13d4bca88
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
ummm, verified fixed. but, suggested to add warning for wifi hotspot opening.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 868631
Flags: needinfo?(philipp)
clear ni?
Flags: needinfo?(dscravaglieri)
You need to log in before you can comment on or make changes to this bug.