Closed Bug 1629875 Opened 4 years ago Closed 4 years ago

Style blocked network messages in Console

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(firefox79 verified)

VERIFIED FIXED
Firefox 79
Tracking Status
firefox79 --- verified

People

(Reporter: nchevobbe, Assigned: kartikc.918, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(4 files, 3 obsolete files)

If you add a request blocking filter in the netmonitor and then fire a request that will be blocked, the requests is styled nicely in the netmonitor to indicate something happened.
It would be nice to do the same in the console. At the moment, there's nothing to distinguish the blocked requests from regular ones.

if someone wants to work on this, I published a small WIP https://phabricator.services.mozilla.com/D70856

it still lacks a few things, like adding the icon, add localization when possible and add some tests to make sure this works as expected.

Maybe a good start without adding the icon is to color-code it like error logs?

Hi Nicolas, I can work on this.
Could you link to where the Netmonitor gets its "blocked" icon from? Do we plan to use it as a globally shared resource or localize it in console (if not already) ?
Also I agree with Harald to color code it red like an error log.
Links and sources would be appreciated.
Thanks!

Hello KC, the bug is now yours :) Thanks for offering help!

Here's how the netmonitor is using the icon: devtools/client/netmonitor/src/assets/styles/StatusCode.css#54-60
We can probably move the image into a parent folder to indicate it's not only used in netmonitor.

We could try the red background, but we need to be careful of the different color combination (in light and dark mode), to make sure the contrast is still good.

This is the component used to display message icon in the console devtools/client/webconsole/components/Output/MessageIcon.js. You can see that we're only having icon depending on the "level" of a message. Here we might want to do something different.
The component is used from the Message component (devtools/client/webconsole/components/Output/Message.js#174-185), so we could change things here. We'd need to check the blockedReason of a message.

This is where we set some of the items for network messages devtools/client/webconsole/components/Output/message-types/NetworkEventMessage.js, we might need to change some things.

Let me know if you have any question :)

Assignee: nobody → kartik.c918
Status: NEW → ASSIGNED

Thanks for assigning this to me. I have a few questions before I start off on this.

I would appreciate some more info on what "levels" decide in devtools/client/webconsole/components/Output/MessageIcon.js , specifically how an icon is selected to be rendered according to the level input.

Also the current implementation I have in mind from your comment is checking against blockedReason and rendering the blocked icon
before the XHR tag. Then go forward with the red error log bg for the message.

Correct me if I'm wrong somewhere, thanks.

This function is one part: https://searchfox.org/mozilla-central/rev/97cb0a90bd053de87cd1ab7646d5565809166bb1/devtools/client/webconsole/components/Output/MessageIcon.js#29-45

For now, it's only used to render logPoint icons by checking the type.
We can probably use that function for the blocked icon. This mean we'd need to pass it the blockedReason IMO (which suppose we also pass it to the MessageIcon component, from the Message one.

The second part (https://searchfox.org/mozilla-central/rev/97cb0a90bd053de87cd1ab7646d5565809166bb1/devtools/client/webconsole/components/Output/MessageIcon.js#56-60) is where we retrieve icons from the level. If there is no type, we're checking in the CONSTANT_ICONS object the one that matches level.

I've added localization for the icon and changed messageIcon.js to render the icon if blockedReason is encountered as type. I have another question.

Since the logged blocked message has a button dropdown to show more information, I was thinking we could render the blocked icon before the "Blocked by Devtools" string. This would preserve the dropdown icon and not be confusing.

Let me know if you think the same would be better.

can you upload screenshots so we can see the two options please?

Attachment #9140426 - Attachment is obsolete: true
Attached image Screenshot of the patch in current state (obsolete) —

This is how the WIP patch looks in the current state. The icon is off by a few pixels vertically that I intend to fix soon.
Let me know what you think.

Thanks,
Kartik

could you try to put the icon at the same position we have other icons (for error messages, info, …) ?

I've already moved it to devtools/client/themes/images/blocked.svg where all the other shared icons for devtools are stored.

Will provide a patch ASAP after the browser toolbox bug is fixed.

Attached image image.png

This is how it looks at 8px icon height, which also solves the vertical alignment issue. Let me know what you think so I can push a final patch soon!

Attachment #9144568 - Attachment is obsolete: true
Flags: needinfo?(nchevobbe)

as said in the review, let's try to put the icon at the same place we put other message icons

Flags: needinfo?(nchevobbe)
Attached image image.png (obsolete) —

I'm having trouble with getting the icon to render at the start of the error log. Pushing the icon through the message component seems to "break it" (check screenshot).

Would appreciate a little help on this, possibly an existing implementation I could learn from.

Thanks!

Flags: needinfo?(nchevobbe)

The error message isn't related to your patch, this is an issue related to the blocking feature.

I tried to guide you in the patch, hope this will unblock you.

Flags: needinfo?(nchevobbe)
Attached image test.png

I accidentally sent the picture of the error below the one I was supposed to in my last comment, don't know how that happened, sorry.

Also, the icon renders in the beginning of the message now but there's an uneven amount of margin (check screenshot).

Should I update CSS accordingly?
Thanks.

Attachment #9152970 - Attachment is obsolete: true

(In reply to KC from comment #18)

Created attachment 9154763 [details]
test.png

I accidentally sent the picture of the error below the one I was supposed to in my last comment, don't know how that happened, sorry.

Also, the icon renders in the beginning of the message now but there's an uneven amount of margin (check screenshot).

Should I update CSS accordingly?
Thanks.

looks good! Yes, please update the CSS to align this new icon with the existing ones (you can have icons by emitting console.error and console.warn messages)

Attachment #9142320 - Attachment description: Bug 1629875 - [WIP] Style blocked network messages in console. → Bug 1629875 - Style blocked network messages in console. r=nchevobbe
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45bd893da14f
Style blocked network messages in console. r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79
No longer blocks: 1642034

Confirmed lack of styling with 78.0.1 esr.
Verified with 79.0b2 on Windows 10, macOS 10.15.5, ubuntu 18.

Status: RESOLVED → VERIFIED

Added blocked icon to the intro description of Console messages

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: