Closed
Bug 1039449
Opened 10 years ago
Closed 9 years ago
Show and edit WiFi debugging device name
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
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.
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8576944 -
Flags: review?(janx)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment 2•9 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•9 years ago
|
Summary: Show WiFi debugging device name → Show and edit WiFi debugging device name
Assignee | ||
Comment 3•9 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•9 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•9 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•9 years ago
|
Attachment #8576944 -
Flags: review- → review?(janx)
Comment 6•9 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•9 years ago
|
||
Proof of the empty DevTools Wi-Fi Device Name problem.
Updated•9 years ago
|
Attachment #8576944 -
Flags: feedback+
Comment 8•9 years ago
|
||
(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•9 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.
Assignee | ||
Comment 10•9 years ago
|
||
/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)
Assignee | ||
Comment 11•9 years ago
|
||
(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•9 years ago
|
Attachment #8576944 -
Flags: review?(janx)
Assignee | ||
Comment 12•9 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93217a4b65c0
Comment 13•9 years ago
|
||
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 14•9 years ago
|
||
Comment on attachment 8579481 [details]
MozReview Request: bz://1039449/jryans
Works like a charm. Thanks!
Attachment #8579481 -
Flags: review?(janx) → review+
Assignee | ||
Comment 15•9 years ago
|
||
Landed Gecko portion: https://hg.mozilla.org/integration/fx-team/rev/11c3430c78f8
Assignee | ||
Comment 16•9 years ago
|
||
Merged Gaia portion after fixing nits: https://github.com/mozilla-b2g/gaia/commit/3c363fd9c3d5db46f24baef4853201325042b946
https://hg.mozilla.org/mozilla-central/rev/11c3430c78f8
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
Assignee | ||
Comment 18•9 years ago
|
||
Attachment #8579481 -
Attachment is obsolete: true
Attachment #8618228 -
Flags: review+
Assignee | ||
Comment 19•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•