Closed Bug 1039449 Opened 6 years ago Closed 6 years ago

Show and edit WiFi debugging device name

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox39 fixed)

RESOLVED FIXED
2.2 S8 (20mar)
Tracking Status
firefox39 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

Details

Attachments

(3 files, 1 obsolete file)

In bug 1027787, we generate a unique device name to use with DevTools discovery.

We should display that name in the Developer menu.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment on attachment 8576944 [details] [review]
[gaia] jryans:device-name > mozilla-b2g:master

The developer menu is not the place for that. If you just want to display the unique device name used for Wi-Fi discovery, please put it in apps/settings/elements/about_more_info.html (e.g. next to "Bluetooth Address").

Also, your change made the device name editable. Was that on purpose? If so, I'm not sure that's the right way to do it. Users would love to customize their device's name, but it should be consistent everywhere (on the phone, in WebIDE, ADB shell, ...) or at least across DevTools, because it's confusing if you rename your device to, say "Jack's Fox" for Wi-Fi discovery, but it still shows up as "flame" in USB devices.
Attachment #8576944 - Flags: review?(janx) → review-
Summary: Show WiFi debugging device name → Show and edit WiFi debugging device name
(In reply to Jan Keromnes [:janx] from comment #2)
> Comment on attachment 8576944 [details] [review]
> [gaia] jryans:device-name > mozilla-b2g:master
> 
> The developer menu is not the place for that. If you just want to display
> the unique device name used for Wi-Fi discovery, please put it in
> apps/settings/elements/about_more_info.html (e.g. next to "Bluetooth
> Address").

Hmm, what if I include it there *as well*, so it's in both places?  To me, it's a bit more obvious that I would change it in Developer, since it only affects DevTools.  But, that's because I know how the feature works, so perhaps I am biased. :)

> Also, your change made the device name editable. Was that on purpose? If so,
> I'm not sure that's the right way to do it. Users would love to customize
> their device's name, but it should be consistent everywhere (on the phone,
> in WebIDE, ADB shell, ...) or at least across DevTools, because it's
> confusing if you rename your device to, say "Jack's Fox" for Wi-Fi
> discovery, but it still shows up as "flame" in USB devices.

Yes, the name is meant to be editable.  I updated the bug title to clarify.

To my knowledge (which is admittedly quite limited about Gaia...), there's no concept of a "global, user-editable device name" yet.  We could decide this is where we start having one, but that sounds like a larger decision that could take a while to resolve i.e. where do you set it, update other uses, etc.

So, maybe we can do this for now, and unify names later?  

As for the ADB name, we would need to change more things for that to be possible.  ADB Helper[1] reads the name from "ro.product.model".  The update service also reads[2] this field, so storing a user-entered name would likely break updates.

[1]: https://github.com/mozilla/adbhelper/blob/master/main.js#L87
[2]: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#332
Flags: needinfo?(janx)
A global user-editable device name would be nice in Firefox OS, but that's out of scope.

A devtools-wide vanity name for your device would be nice (and its pref could indeed be `devtools.discovery.device`). Maybe there is a way to display that vanity name in WebIDE's USB runtime list instead of `ro.product.model`? E.g. by making the helper read both names? The command `adb devices` would still show something like "flame", but at least in WebIDE we'd be consistent across USB and Wi-Fi.

Failing to do that, the custom name should really be called something specific like "DevTools Wi-Fi name". Also this additional input field depending on Wi-Fi visibility is weird, maybe we'll eventually want to move these options to a "Remote debugging" (or "DevTools via Wi-Fi") sub-panel, then we could also show the list of saved certificates there.
Flags: needinfo?(janx)
(In reply to Jan Keromnes [:janx] from comment #4)
> A global user-editable device name would be nice in Firefox OS, but that's
> out of scope.
> 
> A devtools-wide vanity name for your device would be nice (and its pref
> could indeed be `devtools.discovery.device`). Maybe there is a way to
> display that vanity name in WebIDE's USB runtime list instead of
> `ro.product.model`? E.g. by making the helper read both names? The command
> `adb devices` would still show something like "flame", but at least in
> WebIDE we'd be consistent across USB and Wi-Fi.

Yes, this sounds like a good improvement.  I'll do it in a follow up, as we can't read the current setting easily over ADB, so I'll synchronize it with a device property (readable with getprop).

> Failing to do that, the custom name should really be called something
> specific like "DevTools Wi-Fi name".

Makes sense, I'll do this for now.

> Also this additional input field
> depending on Wi-Fi visibility is weird, maybe we'll eventually want to move
> these options to a "Remote debugging" (or "DevTools via Wi-Fi") sub-panel,
> then we could also show the list of saved certificates there.

Yes, we may eventually want a sub-panel.  I think the visibility makes sense, since it only affect Wi-Fi for now.  But note, it's not dependent on whether Wi-Fi is *enabled*, but only if the Wi-Fi option is *visible*.  This will be true by default quite soon.
Attachment #8576944 - Flags: review- → review?(janx)
Comment on attachment 8576944 [details] [review]
[gaia] jryans:device-name > mozilla-b2g:master

Thanks for fixing the Title Case in the Developer Menu! It does improve consistency within the Settings (and makes the Developer Menu more like th iPhone's than Android's).

A few things before I can r+:

Normally string changes need to be accompanied by a l10n-id change, but since you're only changing capitalization (and it couldn't be done with CSS), I don't know. Staś, what's the right thing to do here? Change all the IDs? Send an email to the localizers?

Also, since you're reworking the Developer Menu a bit, please take the opportunity to move the "Pseudo-Localization" check-box to the top of the "Debug" section, because it's more an option to debug than an actual developer tool.

Furthermore, I noticed what could be a bug:
1. `make reset-gaia`
2. Go through FTU, and join the same Wi-Fi network your computer is on
3. Set Debugging via USB to ADB and DevTools
4. Connect with WebIDE, request higher privileges (device reboots)
5. Reconnect with WebIDE, enable Device Settings > devtools.remote.visible

Actual:
6. On the device, DevTools Wi-Fi Device Name is empty
7. When you enable DevTools via Wi-Fi, WebIDE sees "flame-8f8d4a84"
(8. Killing and restarting Settings fixes Wi-Fi Device Name)

Expected:
6. DevTools Wi-Fi Device Name should be correct directly
Flags: needinfo?(stas)
Attachment #8576944 - Flags: review?(janx)
Attached image wifi-name-problem.jpg
Proof of the empty DevTools Wi-Fi Device Name problem.
Attachment #8576944 - Flags: feedback+
(In reply to Jan Keromnes [:janx] from comment #6)

> Normally string changes need to be accompanied by a l10n-id change, but
> since you're only changing capitalization (and it couldn't be done with
> CSS), I don't know. Staś, what's the right thing to do here? Change all the
> IDs? Send an email to the localizers?

Thanks for looping me in!  In case of capitalization changes, it's recommended to not change the l10n id.  Languages other than English may have their own rules about capitailization;  for instance they might not be using the title case at all.

It looks like in the pull request one of the reasons to change the l10n id is consistency (dev-tools → devtools), which I think is okay, but keep in mind that it will cause some work for the localizers who will need to update their files, so maybe better just leave it as is?
Flags: needinfo?(stas)
(In reply to Staś Małolepszy :stas from comment #8)
> (In reply to Jan Keromnes [:janx] from comment #6)
> 
> > Normally string changes need to be accompanied by a l10n-id change, but
> > since you're only changing capitalization (and it couldn't be done with
> > CSS), I don't know. Staś, what's the right thing to do here? Change all the
> > IDs? Send an email to the localizers?
> 
> Thanks for looping me in!  In case of capitalization changes, it's
> recommended to not change the l10n id.  Languages other than English may
> have their own rules about capitailization;  for instance they might not be
> using the title case at all.

Right, that's my understanding too, so I've left the IDs alone generally.

> It looks like in the pull request one of the reasons to change the l10n id
> is consistency (dev-tools → devtools), which I think is okay, but keep in
> mind that it will cause some work for the localizers who will need to update
> their files, so maybe better just leave it as is?

This only happened for 2 l10n IDs, but it is extra localizer work, it's true.
Attached file MozReview Request: bz://1039449/jryans (obsolete) —
/r/5641 - Bug 1039449 - Init Wi-Fi device name at startup. r=janx

Pull down this commit:

hg pull review -r f4914f64000e80f112f6951bb64b9791c935f0b6
Attachment #8579481 - Flags: review?(janx)
(In reply to Jan Keromnes [:janx] from comment #6)
> Also, since you're reworking the Developer Menu a bit, please take the
> opportunity to move the "Pseudo-Localization" check-box to the top of the
> "Debug" section, because it's more an option to debug than an actual
> developer tool.

Okay, I've done this.

> Furthermore, I noticed what could be a bug:
> 1. `make reset-gaia`
> 2. Go through FTU, and join the same Wi-Fi network your computer is on
> 3. Set Debugging via USB to ADB and DevTools
> 4. Connect with WebIDE, request higher privileges (device reboots)
> 5. Reconnect with WebIDE, enable Device Settings > devtools.remote.visible
> 
> Actual:
> 6. On the device, DevTools Wi-Fi Device Name is empty
> 7. When you enable DevTools via Wi-Fi, WebIDE sees "flame-8f8d4a84"
> (8. Killing and restarting Settings fixes Wi-Fi Device Name)
> 
> Expected:
> 6. DevTools Wi-Fi Device Name should be correct directly

I think I never noticed this since I've been testing with Wi-Fi enabled by default... :) Anyway, I never seem to get a settings change event when the device name is first generated.  So, to work around this, I force the generation to happen at startup in a separate Gecko patch for you to review.
Comment on attachment 8576944 [details] [review]
[gaia] jryans:device-name > mozilla-b2g:master

Looking good! One nit and a conflict to fix, and you're good to go.

Thanks for all the great work!
Attachment #8576944 - Flags: review?(janx) → review+
Comment on attachment 8579481 [details]
MozReview Request: bz://1039449/jryans

Works like a charm. Thanks!
Attachment #8579481 - Flags: review?(janx) → review+
https://hg.mozilla.org/mozilla-central/rev/11c3430c78f8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
Attachment #8579481 - Attachment is obsolete: true
Attachment #8618228 - Flags: review+
Depends on: 1182715
You need to log in before you can comment on or make changes to this bug.