Although we warn about this in the console, we do not show anything in the network monitor.

Splitting this out from a bug that's confidential, as these bits don't need to be. Relevant bits of discussion:

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #3)

(In reply to :Gijs (he/him) from comment #1)

We can probably add some notification to the network tab to list that access to the CORS request from JS is blocked, in addition to the console warnings around this, but we should avoid doing exactly what Chrome is doing and show the same kind of "blocked" UI that we do when the request actually does not make it to the server (due to CSP, or an add-on blocking requests, or some other reason).

Yes, agree.

(In reply to :Gijs (he/him) from comment #2)

The devtools side of this looks like a regression, cf. bug 1638313. Honza?

That bug only added platform support to identify requests "blocked" by CORS. The DevTools side wasn't completed (see bug 1640099).

So, let's use this bug to implement the notification.

1. We could use similar UI as suggested for exposing anti-tracking classification flag in bug 1537627
2. Or we could show an icon indicating "this request is inaccessible due to CORS" in the Status column (the same place where we show the Request Blocked icon)
3. Or both

Honza

I am attaching a mockup of how the UI should look like.

Honza

I would be interested in working on this issue. Do you know which files I should look in to add the new functionality and UI? Also, to clarify this bug isn't about showing cross-origin blocking in the summary panel but adding more details about the blocking to the headers panel when you click for more info on the blocked request.

Mattias

I implemented the initial functionality of adding a message in the headers panel that the request was blocked. Right now it will add the message from BLOCKED_REASON_MESSAGES in devtools/client/netmonitor/src/constants.js for all blocked requests but I'm not sure if it's better to limit the scope right now to just requests blocked by CORS.

I was also wondering if there is a premade component or similar for rendering the warning box or if I should write my own.

Mattias

Thanks starting on this Mattias!

I implemented the initial functionality of adding a message in the headers panel that the request was blocked. Right now it will add the message from BLOCKED_REASON_MESSAGES in devtools/client/netmonitor/src/constants.js for all blocked requests but I'm not sure if it's better to limit the scope right now to just requests blocked by CORS.

We only the need the notification for requests blocked by CORS, so lets limit it to that. I'm not sure if we want it for all types of CORS errors but we can look at that later.

I was also wondering if there is a premade component or similar for rendering the warning box or if I should write my own.

You can use this NotificationBox https://searchfox.org/mozilla-central/search?q=NotificationBox&redirect=false

Feel free to open an in-progress patch.

What is the correct way to apply styles to a component in the Devtools codebase? I've been able to get the NotificationBox to display but I'm struggling to get it styled and many online resources assume that you are using webpack.

Mattias

Hi Mattias,
You can add the NotificationBox style sheet here https://searchfox.org/mozilla-central/rev/6371054f6260a5f8844846439297547f7cfeeedd/devtools/client/netmonitor/src/assets/styles/netmonitor.css#5-14

@import "chrome://devtools/content/shared/components/NotificationBox.css"

Then you can reference the classes from one of the other netmonitor stylesheets. I guess this might be assest/styles/HeadersPanel.css.
Hope this helps

Also feel free to open a work in progress patch and it's easier to help when looking at the code.
Thanks

Depends on D114377

I want to write a test for the new UI but I'm unsure how to simulate a request blocked by CORS so that it has the correct blocked reason. Is there a good way to emulate a CORS blocked request in the test setup?

Mattias

Hi Mattias,
Here is a simple cors test https://searchfox.org/mozilla-central/rev/3151f97de27730793c2e298716df760999423f26/devtools/client/netmonitor/test/browser_net_cors_requests.js

Thanks

Related webcompat issue filed https://webcompat.com/issues/72813

Here is what the UI is looking like right now. I'm not sure how well the NotificationBox works for two reasons. First, I don't know if the warning should be closable. Second, at least in the way I was doing it, I couldn't reuse the existing devtools learn-more link code and had to partially reimplement it to get a learn more button.

Mattias

This looks great Mattias, thank you!

(In reply to Mattias de los Rios Rogers [:delosrogers] from comment #14)

First, I don't know if the warning should be closable.

Good point. Agree, the close button should not be there.
I think we could introduce new displayCloseButton prop in the NotificationBox component (true by default)

Second, at least in the way I was doing it, I couldn't reuse the existing devtools learn-more link code and had to partially reimplement it to get a learn more button.

Perhaps we could move this piece directly to the NotificationBox and introduce displayLearnMore prop. If MDNLink component can't be directly reused we can adjust/polish this piece in a follow up bug.

Honza

Ok, I'll start working on adding a displayCloseButton prop and a displayLearnMore prop. I think that I'll also have to add a displayButtons prop because as the component is right now it will always display at least one general-purpose button (even if you pass an empty array of buttons) and I think that the learn-more link should be separate.

Mattias

Here is the new UI using the MDNLink component as a learn more link and without a close button. I think looks much better and it's nice that it matches matches the learn more link of the status code. Also, I was wrong about needing a displayButtons prop because NotificationBox actually doesn't display any buttons when you pass an empty array, I'm not sure why I thought it did.

Mattias

It looks great!

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:delosrogers, could you have a look please?
For more information, please visit auto_nag documentation.

Bomsy, please test, but it looks like we can land the patches.
Thank you!

https://hg.mozilla.org/mozilla-central/rev/810afa9a4fcd
https://hg.mozilla.org/mozilla-central/rev/770b763ba0fd