1404888 - Migrate browser_webconsole_bug_762593_insecure_passwords_web_console_warning.js to the new frontend

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

The test needed to be rewritten as we can't have the 3 warning messages
at the same time (see https://hg.mozilla.org/mozilla-central/rev/084f217c4d49#l1.76
and https://hg.mozilla.org/mozilla-central/diff/817eb00ba702/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm#l1.106).
This wasn't picked up by the old frontend test because it is disabled.
In order to test the different warning messages we open 3 different pages
which log the appropriate warning.

Review commit: https://reviewboard.mozilla.org/r/215252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215252/

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

https://reviewboard.mozilla.org/r/215252/#review220902

Thanks!

Some copy pasting between step 2 and 3  but the approach looks good.
If try is green after fixing the last part, feel free to land without re-review.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_insecure_passwords_web_console_warning.js:15
(Diff revision 1)
>  
>  const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
> -                 "test/test-bug-762593-insecure-passwords-web-" +
> -                 "console-warning.html";
> -const INSECURE_PASSWORD_MSG = "Password fields present on an insecure " +
> -                 "(http://) page. This is a security risk that allows user " +
> +  "new-console-output/test/mochitest/test-insecure-passwords-web-console-warning.html";
> +const TEST_URI_2 = "http://example.com/browser/devtools/client/webconsole/" +
> +  "new-console-output/test/mochitest/test-iframe-insecure-form-action.html";
> +const TEST_URI_3 = "https://example.com/browser/devtools/client/webconsole/" +

TEST_URI_3 is the same as TEST_URI_2

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_insecure_passwords_web_console_warning.js:41
(Diff revision 1)
> -        name: "Insecure iframe error displayed successfully",
> -        text: INSECURE_IFRAME_MSG,
> -        category: CATEGORY_SECURITY,
> -        severity: SEVERITY_WARNING
> -      },
> +  hud = await openNewTabAndConsole(TEST_URI_3);
> +  message = await waitFor(()=>
> +    findMessage(hud, INSECURE_FORM_ACTION_MSG, ".message.warn"), "", 100);
> +  ok(message, "Insecure form action error displayed successfully");
> +  await testLearnMoreLinkClick(message, INSECURE_PASSWORDS_URI);

This is the same as the test for TEST_URI_2

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

https://reviewboard.mozilla.org/r/215252/#review220902

> TEST_URI_3 is the same as TEST_URI_2

TEST_URI_3 is https, TEST_URI_2 is http

> This is the same as the test for TEST_URI_2

not really, we wait for INSECURE_FORM_ACTION message (INSECURE_IFRAME_MSG in TEST_URI_2)

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

https://reviewboard.mozilla.org/r/215252/#review220952

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_insecure_passwords_web_console_warning.js:15
(Diff revision 1)
>  
>  const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
> -                 "test/test-bug-762593-insecure-passwords-web-" +
> -                 "console-warning.html";
> -const INSECURE_PASSWORD_MSG = "Password fields present on an insecure " +
> -                 "(http://) page. This is a security risk that allows user " +
> +  "new-console-output/test/mochitest/test-insecure-passwords-web-console-warning.html";
> +const TEST_URI_2 = "http://example.com/browser/devtools/client/webconsole/" +
> +  "new-console-output/test/mochitest/test-iframe-insecure-form-action.html";
> +const TEST_URI_3 = "https://example.com/browser/devtools/client/webconsole/" +

My bad! I did read several times so I think it couldn't hurt to make the difference more obvious. With meaningful names maybe?

Same for the tests below, they're so similar that the boilerplate code makes it hard to spot differences. Mutualize in a test function where we just pass the URI and expected message?

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Good ideas, i'll rename constants and use a generic function.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215252/diff/1-2/

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

Clearing review flag since things changed a bit

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8945037 [details]
Bug 1404888 - Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; .

That looks great! Sorry for misreading the first patch, but in the end I'm glad we are landing this version, thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e4b0027e470
Enable browser_webconsole_insecure_passwords_web_console_warning.js in new console frontend; r=jdescottes.

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/3e4b0027e470