1629875 - Style blocked network messages in Console

Status: VERIFIED FIXED

(DevTools :: Console, enhancement, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Screenshot at Apr 14 15-27-05.png

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.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1629875 - [WIP] Style blocked network messages in console.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 3

[:Harald Kirschner :digitarald]

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

Comment 4

[KC]

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!

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

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 :)

Comment 6

[KC]

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.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 8

[KC]

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.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 10

[KC]

Attachment: Bug 1629875 - Style blocked network messages in console. r=nchevobbe

Comment 11

[KC]

Attachment: Screenshot of the patch in current state

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

Comment 12

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 13

[KC]

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.

Comment 14

[KC]

Attachment: 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!

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 16

[KC]

Attachment: image.png

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!

Comment 17

[Nicolas Chevobbe [: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.

Comment 18

[KC]

Attachment: 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.

Comment 19

[Nicolas Chevobbe [:nchevobbe]]

(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)

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45bd893da14f
Style blocked network messages in console. r=nchevobbe

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/45bd893da14f

Comment 22

[Cristian Fogel, QA [:cfogel]]

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

Comment 23

[Janet Swisher]

Added blocked icon to the intro description of Console messages