Add learn more link

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Console
P2
enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: linclark, Assigned: nchevobbe)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: new-console)

MozReview Requests

()

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

Attachments

(1 attachment)

(Reporter)

Description

a year ago
Originally posted by:linclark

see https://github.com/devtools-html/gecko-dev/issues/229

Currently in the web console we show learn more links. we need to add these to things like mixed content warnings.

Manual testing page:
- https://googlesamples.github.io/web-fundamentals/samples/fundamentals/security/prevent-mixed-content/image-gallery-example.html
(Reporter)

Updated

a year ago
Priority: -- → P2
Whiteboard: new-console
(Reporter)

Updated

a year ago
Blocks: 1308219
(Assignee)

Updated

a year ago
Assignee: nobody → chevobbe.nicolas
Comment hidden (mozreview-request)
(Reporter)

Comment 2

a year ago
mozreview-review
Comment on attachment 8801688 [details]
Bug 1307919 - Add "Learn more" MDN link in message output. ;

https://reviewboard.mozilla.org/r/86298/#review85252

I haven't had a chance to manually test this yet (waiting on a build), but overall it looks good to me. Just a few comments below. I'm marking as r- because I think there will be a warning about keys, but that's easy to fix.

::: devtools/client/webconsole/new-console-output/components/message-types/console-command.js:38
(Diff revision 1)
>    const {
>      source,
>      type,
>      level,
>      messageText: messageBody,
> +    exceptionDocURL,

Will console commands ever have an exception doc URL?

::: devtools/client/webconsole/new-console-output/components/message.js:165
(Diff revision 1)
>        icon,
>        collapse,
>        dom.span({ className: "message-body-wrapper" },
>          dom.span({ className: "message-flex-body" },
>            dom.span({ className: "message-body devtools-monospace" },
> -            messageBody
> +            [messageBody, learnMore]

You shouldn't need to make this an array. If you make this an array, it likely sets off React's warning about keys. Instead, you can just add learnMore as an additional parameter (as repeat and location are for the parent span).

::: devtools/client/webconsole/new-console-output/test/components/evaluation-result.test.js:48
(Diff revision 1)
>      expect(wrapper.find(".message-body").text())
> -      .toBe("ReferenceError: asdf is not defined");
> +      .toBe("ReferenceError: asdf is not defined[Learn More]");
>  
>      expect(wrapper.find(".message.error").length).toBe(1);
> +
> +    // There should be a [Learn more] link.

Maybe we should break this out into a separate it() statement?

::: devtools/client/webconsole/new-console-output/test/components/page-error.test.js:51
(Diff revision 1)
>      const locationLink = wrapper.find(`.message-location`);
>      expect(locationLink.length).toBe(1);
>      // @TODO Will likely change. See https://github.com/devtools-html/gecko-dev/issues/285
>      expect(locationLink.text()).toBe("test-tempfile.js:3:5");
> +
> +    // There should be a [Learn more] link.

Ditto.
Attachment #8801688 - Flags: review?(lclark) → review-
(Reporter)

Comment 3

a year ago
I just manually tested and it looks good to me. With the fixes, this will be an r+.
(Assignee)

Comment 4

a year ago
mozreview-review-reply
Comment on attachment 8801688 [details]
Bug 1307919 - Add "Learn more" MDN link in message output. ;

https://reviewboard.mozilla.org/r/86298/#review85252

> Will console commands ever have an exception doc URL?

No you're right. Forgot to clean this one

> Maybe we should break this out into a separate it() statement?

yes, it makes sense
Comment hidden (mozreview-request)
(Assignee)

Comment 6

a year ago
TRY looks fine https://treeherder.mozilla.org/#/jobs?repo=try&revision=bab07a6334d8
(Reporter)

Comment 7

a year ago
mozreview-review
Comment on attachment 8801688 [details]
Bug 1307919 - Add "Learn more" MDN link in message output. ;

https://reviewboard.mozilla.org/r/86298/#review85716

Awesome! Bit by bit we're getting closer to dev edition. Thanks for your work on this.
Attachment #8801688 - Flags: review?(lclark) → review+

Comment 8

a year ago
Pushed by chevobbe.nicolas@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e2511b7cd8d9
Add "Learn more" MDN link in message output. r=linclark;

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e2511b7cd8d9
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.