@StepanStava if you need more time to investigate usage of requestIdleCallback, I would not block this bug on it – as the feature is optional and fast rFA-triggered logging isn't common (vs just sprinkling in some logs).

In D81678#2535965, @StepanStava wrote:

Could you please point me to an example in the codebase where it's used. Thank you

As this is low UX risk and matches perfectly what Victoria provides, this can pass without extra UX review.

LGTM. Thanks for reducing the chance for users to hit this issue.

In D83319#2562219, @jdescottes wrote:

@Harald @Honza thoughts on the wording?
Anything else we should mention? (eg "DevTools should be restarted afterwards.")

In D81678#2527948, @nchevobbe wrote:

I saw a few issue:

This does only seem to be exposed in the source context menu. Could we also expose it in the overflow settings?

In D76527#2431773, @mayhemer wrote:

In D76527#2431770, @mayhemer wrote:

@mayhemer you can see the function we use to retrieve a possible MDN URL for a given error: https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/devtools/server/actors/errordocs.js#164-193
I guess we either want to link to https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types/preload or directly to https://developer.mozilla.org/en-US/docs/Web/HTML/Preloading_content
Let me know if it's not clear how to proceed

Thanks for the reference @nchevobbe but I'm not sure what to do at all. I assume I should add a new LEARN_MORE somewhere here and then use it somewhere, somehow - no idea how.

Somehow like this?

The outcome of this logic seems right, but Jason can help with reviewing the approach.

In D76527#2335632, @flod wrote:

In D76527#2335602, @Harald wrote:

Could we file a follow-up bug to add an MDN link for this message as well?

Do you mean a link as part of the string or something else?

Error message looks ok, close enough to what Chrome has. Adding @nchevobbe for the console related parts.

Accepted with minor revision on adding context.

r+ on UI.

In D74535#2277875, @Honza wrote:

It looks good code-wise, thanks for working on this Kriyszig!

@Harald, what if the Blocking panel displayed a message (gray color, like other messages in empty panels) saying something like "Drag and Block"?

Honza

Seems like a good idea … we usually don't have "Open" on variables, but inspect/log/preview/watch.

I think the curly underline looks great and clean. Here's a screenshot.

I filed https://github.com/firefox-devtools/ux/issues/124 for UX discussion.

In D71365#2211882, @davidwalsh wrote:

@victoria @Harald I think we should consider some type of UI notifier for when JavaScript is turned off. Maybe we color the icon red in that case?

In D71037#2199494, @jdescottes wrote:

Harald: this is something we discussed but I don't think we had a clear conclusion. Do we want to force the popup on users who already are marked as "DevTools users"?

Adding Victoria for UX sign off.

In D72183#2195390, @jlast wrote:

why get rid of Inspect & Debug Service Workers

I still need to move this to the appropriate spot in the UI, hoping maybe @victoria or @Harald will add their 2 cents about where it could go.

@Honza looks like things match. Should we approve this minor change then?

r+ with the new string we discussed.

Adding Victoria to review the wording.

In D68072#2069140, @davidwalsh wrote:

This patch does *not* modify the preference to *keep* the panels open -- it only opens the panel if we break on the given breakpoint type. This is best case scenario IMO -- we show the user relevant info but don't change their preference.

LGTM from UI side. Nice CSS cleanup.

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1566780#c15 , not sure if we should overload the select with a mix of multi-select and single-select behaviour – that might be hard to explain. Maybe @victoria has ideas on how to solve this. Could you try adding a toggle in the toolbar for Control?

I don't know why we chose to use a monospace font here. @Harald might know?
Personally I'd be okay with using the standard, variable-width font for this list. Its consistent with the search UI and with how we display URLs in the main request list. Added bonus: it lets us show more characters per line.

LGTM, thank you!

Reducing reviewers as Nicolas is a good peer for this.

Nice work @transfusion !

Great, let's land this tomorrow after the merge day.

LGTM on UI

Good on fix. I have not worked too much on this code, so will defer To Honza for code review.

LGTM, even fantastic. Thank you!

LGTM feature-wise.

In D65077#1985778, @FarooqAR wrote:

In D65077#1980512, @Harald wrote:

Title on the drag element could help.

@Harald Should we add the title in this patch or file another bug?

Waiting to experiment further with this.

In D63799#1978027, @Honza wrote:

So, I think the CM size should always be limited by the size of the panel, i.e. occupy only the available area. This leads to a point where the Response panel should actually have just one Accordion section opened at a time (JSON preview or Response payload). We do it by default already. WDYT?

You'll need to rebase to not override https://phabricator.services.mozilla.com/D64841.

I like it so, yes if it's ok with Harald.

Do you want to add double-click-resizer functionality in a separate bug or cover it here?

I got a failed git command the first time; but it still submitted.

Thanks for the quick work @outloudvi!

@jlast, this seems to make the paused line more flaky, or at least not detectable in tests (notice the confused "failed" result)

Thank you, @victoria for the polish!

In D63020#1926260, @Honza wrote:

Checking glob and other pattern matching libs, * matches empty strings as well. It would only affect edge cases; probably (as I can’t come with an example where it would be a foot fun); but might be more expected to use .*.

@Harald I don't understand this comment/reasoning. Now there is no difference between *test and test and test* blocking patterns

I thought that the differentiator is: test* means => everything that contains "test" followed by 1+ characters

Honza

In D28274#1921726, @Honza wrote:>

E.g. there are multiple trees in the Params panel and making the first item selected in every one of them might be confusing.
It's doesn't have to be clear where the focus is...

@Harald, WDYT?

Thanks for the quick fixes!

LGTM, thanks a lot for the work and the quick tweaks!