Show and edit WiFi debugging device name

RESOLVED FIXED in Firefox 39

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

(Depends on: 1 bug)

unspecified
2.2 S8 (20mar)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

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

We should display that name in the Developer menu.
Created attachment 8576944 [details] [review]
[gaia] jryans:device-name > mozilla-b2g:master
(Assignee)

Updated

3 years ago
Attachment #8576944 - Flags: review?(janx)
(Assignee)

Updated

3 years ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED

Comment 2

3 years ago
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-
(Assignee)

Updated

3 years ago
Summary: Show WiFi debugging device name → Show and edit WiFi debugging device name
(Assignee)

Comment 3

3 years ago
(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)

Comment 4

3 years ago
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)
(Assignee)

Comment 5

3 years ago
(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.
(Assignee)

Updated

3 years ago
Attachment #8576944 - Flags: review- → review?(janx)

Comment 6

3 years ago
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)

Comment 7

3 years ago
Created attachment 8579333 [details]
wifi-name-problem.jpg

Proof of the empty DevTools Wi-Fi Device Name problem.

Updated

3 years ago
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)
(Assignee)

Comment 9

3 years ago
(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.
Created attachment 8579481 [details]
MozReview Request: bz://1039449/jryans

/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.
(Assignee)

Updated

3 years ago
Attachment #8576944 - Flags: review?(janx)
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
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
Comment on attachment 8579481 [details]
MozReview Request: bz://1039449/jryans
Attachment #8579481 - Attachment is obsolete: true
Attachment #8618228 - Flags: review+
Created attachment 8618228 [details]
MozReview Request: Bug 1039449 - Init Wi-Fi device name at startup. r=janx
Depends on: 1182715
You need to log in before you can comment on or make changes to this bug.