Closed Bug 1145221 Opened 5 years ago Closed 5 years ago

Add "always allow" option for WiFi debugging

Categories

(Firefox OS Graveyard :: Developer Tools, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jryans, Assigned: jryans)

References

Details

Attachments

(1 file)

The WiFi debugging connection prompt should offer a "remember / always allow" option to scan once, and then persist the client based on its certificate.

This work began in bug 1032128, but I later realized that bug is larger super-set of the features I was adding.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Blocks: 1032128
Blocks: wifi-rdp
Replying to Jan's feedback originally in bug 1032128:

> - The new explanation text in the prompt is confusing, and it's not my
> suggestion: "other endpoint" is hard to understand, and "You can also
> remember the other endpoint to skip scans in the future" is unnecessary and
> confusing. Please remove the last sentence and don't use "endpoint" or
> "other endpoint".

We discussed this more, and agree to replace "endpoint" with "device", and to change the explanation sentence to: "You can avoid future scans by remembering this device."

> - You didn't address one of the nits I mentioned in comment 23 [1], without
> which the button text is misaligned.

I was testing on the simulator, and I really couldn't tell the difference there.  But, it seems slightly more clear on device.  I'll include the tweaks you mentioned.

> - There was no need to change the "remoteDebuggerPromptUSB" l10n-id to
> "remoteDebuggerPromptUSB.innerHTML" because the string didn't change.

This was required because of the use of innerHTML with the WiFi version.  If you show a WiFi prompt and later show the USB version without innerHTML on both, I got a "setTextContent is deprecated" error from l10n.js[1].  Using innerHTML on both is quick workaround for this.

#l10n on IRC advised me to use a fresh string ID, even though the old patch was only landed for a short time.

> Also, please move the "Scan and Remember" option below the "Scan" option,
> which is a more natural order.

I decided I don't feel that strongly, so I'll change to your suggestion.

> Again, I'm very sorry that I asked Carsten to back you out. Please don't
> take it the wrong way.

No worries, you did the right thing as a reviewer seeing things you disagree with get merged! :) Especially in the case of text changes, where we want to save work for others.

[1]: https://github.com/mozilla-b2g/gaia/commit/c4657a4a69b937cc289e7b035a9499a1772cb344#diff-5ead430f1151d5f18b3c794ae6165b1fR1768
Attachment #8580145 - Flags: review?(janx)
Comment on attachment 8580145 [details] [review]
[gaia] jryans:always-allow > mozilla-b2g:master

Looks good to me! Thank you for addressing everything and getting back to me.

I had to run the tests locally because our github bot forgot to trigger them.
Attachment #8580145 - Flags: review?(janx) → review+
As suggested on IRC, I have made the following changes:

the [-other-]{+remote+} device's certificate

by remembering [-this-]{+the+} device.
Merged:

https://github.com/mozilla-b2g/gaia/commit/3b8e3e084f6ebab130323659f66342f5160edff8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.