1126965 - WiFi auth client dialog for WebIDE

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

With bug 1103120 landed, we should override the WiFi client OOB dialog in WebIDE to show a QR code.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: bz://1126965/jryans

/r/3267 - Bug 1126965 - Show QR code for WiFi auth in WebIDE. r=past

Pull down this commit:

hg pull review -r e8ba4dd40d13f9c77c4551222fc225de4810f752

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d1d92b3b0a9

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8558111 [details]
MozReview Request: bz://1126965/jryans

https://reviewboard.mozilla.org/r/3265/#review2721

::: browser/devtools/webide/content/wifi-auth.js
(Diff revision 1)
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +const Cu = Components.utils;

Y U NO "use strict;"?

::: browser/devtools/webide/content/wifi-auth.js
(Diff revision 1)
> +  window.removeEventListener("load", onLoad);

addEventListener had useCaptrue=true, so removeEventListener should too.

::: browser/devtools/webide/content/wifi-auth.js
(Diff revision 1)
> +  let imgData = QR.encodeToDataURI(oobData, "L" /* low quality */);

Blech, C-style parameter comment.
FINE.

::: browser/devtools/webide/content/wifi-auth.js
(Diff revision 1)
> +  document.querySelector("#close").onclick = () => window.close();
> +  document.querySelector("#no-scanner").onclick = showToken;
> +  document.querySelector("#yes-scanner").onclick = hideToken;

Maybe this file does not contain performance-critical code, but why take the slow, rule-parsing path, instead of the quick getElementById one?

::: browser/devtools/webide/modules/runtimes.js
(Diff revision 1)
> +   * @param host string
> +   *        The host name or IP address of the debugger server.
> +   * @param port number
> +   *        The port number of the debugger server.
> +   * @param cert object (optional)
> +   *        The server's cert details.
> +   * @param authResult AuthenticationResult
> +   *        Authentication result sent from the server.
> +   * @param oob object (optional)
> +   *        The token data to be transferred during OOB_CERT step 8:
> +   *        * sha256: hash(ClientCert)
> +   *        * k     : K(random 128-bit number)
> +   * @return object containing:
> +   *         * close: Function to hide the notification
> +   */
> +  sendOOB(session) {

Maybe clarify the relationship between the |session| argument and the various @param notes in the comment?

::: browser/devtools/webide/themes/wifi-auth.css
(Diff revision 1)
> +  margin-left: 10px;

We use -moz-margin-start in these cases to support RTL locales.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://reviewboard.mozilla.org/r/3265/#review2755

> Maybe this file does not contain performance-critical code, but why take the slow, rule-parsing path, instead of the quick getElementById one?

Good idea, changed to getElementById.

> We use -moz-margin-start in these cases to support RTL locales.

Actually, this line is incorrect to start with, so I've removed it.

> Y U NO "use strict;"?

Added.

> addEventListener had useCaptrue=true, so removeEventListener should too.

Doesn't seem necessary, removed true from aEL instead.

> Maybe clarify the relationship between the |session| argument and the various @param notes in the comment?

Okay, added a line to clarify.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=257b90f29af2

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://hg.mozilla.org/integration/fx-team/rev/a462b35377de

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a462b35377de

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1126965 - Show QR code for WiFi auth in WebIDE. r=past