[blazor][wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app #46132

thaystg · 2023-01-17T03:39:24Z

Error if firefox is not opened with remote debugging enabled on port 6000:

Error if firefox is opened but about:debugging tab is not opened:

Debugging working:

HostedBlazorWebassemblyApp.Mozilla.Firefox.2023-01-17.18-38-33.mp4

The goal of this PR is give the customer a similar experience on firefox that it has when debugging on chrome/edge.

We cannot open automatically the about:debugging tab as we do in chrome/edge for security reasons.
A message is shown on app asking to open about:debugging tab.
Then we connect remotely to this tab and use these functions to configure, connect and open a new tab to debug the blazor app:

AboutDebugging.actions.addNetworkLocation
AboutDebugging.actions.connectRuntime
AboutDebugging.actions.selectPage
AboutDebugging.actions.inspectDebugTarget

I didn't find a way to call await functions using evaluateJSAsync from firefox debug protocol, that is why I need some if's before invoke the functions.

Related to dotnet/runtime#80722

Fixes dotnet/runtime#80043

mkArtakMSFT

Thanks @thaystg.
I see that this was not a trivial code due to being forced to read the content in the browser to determine what needs to happen and when.
While this is an ok approach, it'll be relatively high maintenance, as these strings can change over time. Also, I wonder how this will work for non-english culture OS-es? Are these strings still in english ?

mkArtakMSFT · 2023-01-19T19:24:18Z

src/Components/Web.JS/src/Platform/Mono/MonoDebugger.ts

-  link.target = '_blank';
-  link.rel = 'noopener noreferrer';
-  link.click();
+  if (isFirefox) {


This is a lot of Firefox-specific code. Maybe it's worth to move it into a helper function so it's easier to read the logic here:

if (isFirefox) launchFirefoxDebugger(); else{ ... }

mkArtakMSFT · 2023-01-19T19:24:22Z

src/Components/WebAssembly/Server/src/DebugProxyLauncher.cs

+                    capturedUrl = matchFirefox.Groups["url"].Value;
+                    taskCompletionSource.TrySetResult(capturedUrl);
+                }
+                if (isFirefox)


if the condition in the if block above has evaluated to true isn't that already an indication that isFirefox is true? is there possibility for the check above to evaluate to true but isFirefox to be false ?

I'm just trying to see if it's possible to combine these two if blocks together.

mkArtakMSFT · 2023-01-19T19:26:17Z

src/Components/WebAssembly/Server/src/TargetPickerUi.cs

+#pragma warning disable CA1835
+        static async Task<string> ReceiveMessageLoop(TcpClient browserDebugClientConnect, CancellationToken token)
+        {
+            NetworkStream toStream = browserDebugClientConnect.GetStream();


Shouldn't a using be added to this so the toStream gets disposed after it's not needed any more?

thaystg · 2023-01-19T19:59:51Z

Thanks @thaystg. I see that this was not a trivial code due to being forced to read the content in the browser to determine what needs to happen and when. While this is an ok approach, it'll be relatively high maintenance, as these strings can change over time. Also, I wonder how this will work for non-english culture OS-es? Are these strings still in english ?

Which strings?
Like listTabs? watchTargets? watchResources? They are from the firefox remote debugging protocol.
https://firefox-source-docs.mozilla.org/devtools/backend/protocol.html#remote-debugging-protocol

mkArtakMSFT

Thanks @thaystg !

SteveSandersonMS · 2023-01-19T20:43:45Z

src/Components/Web.JS/src/Platform/Mono/MonoDebugger.ts

+          warningToDebug.style.display = 'none';
+        }, 600);
+      };
+      warningToDebug.appendChild(closeWarningButton);


This is quite a lot of code to include if this is only for displaying an error about debugging. Since we have to include all this code even in production bundles, would it be viable to shrink it? For example we don't need an error to be nicely styled. I think it would be enough even just to use alert(someMessage). Or if it's unlikely this ever occurs, we could just console.error(someMessage).

This is quite a lot of code to include if this is only for displaying an error about debugging. Since we have to include all this code even in production bundles, would it be viable to shrink it? For example we don't need an error to be nicely styled. I think it would be enough even just to use alert(someMessage). Or if it's unlikely this ever occurs, we could just console.error(someMessage).

This will probably happen usually, but if you think a console.warning is better for sure I can change it :)

I would ideally prefer to start with something minimal like output to console (or alert if it's desirable to be visible in the main UI).

SteveSandersonMS · 2023-01-19T20:44:54Z

src/Components/WebAssembly/Server/src/Microsoft.AspNetCore.Components.WebAssembly.Server.csproj

@@ -12,6 +12,7 @@
  </PropertyGroup>

  <ItemGroup>
+    <Reference Include="Newtonsoft.Json" Version="13.0.2" />


Could we avoid adding this dependency? Or if there's some reason why System.Text.Json is insufficient, could you clarify what it is?

This is the same used on BrowserDebugProxy, if you prefer I can try to use System.Text.Json.

Ah, I see, so it won't change the dependencies as far as customers are concerned.

However, having the direct dependency here does mean more dependabot-type noise and TBH it's just really surprising to see Json.NET code show up when we've consistently used STJ elsewhere in this repo. If it's not too disruptive to switch over to STJ for this that would be helpful! If that's impractical or a lot of work let me know.

SteveSandersonMS · 2023-01-19T20:46:38Z

src/Components/WebAssembly/Server/src/TargetPickerUi.cs

+                    else if (!string.IsNullOrEmpty(toCmd))
+                    {
+                        await SendMessageToBrowser(toStream, JObject.FromObject(new { type = "getWatcher", isServerTargetSwitchingEnabled = true, to = toCmd }), token);
+                    }


Is this definitely the right way to factor the code? Since debugging-WebAssembly-on-Firefox isn't Blazor-specific (it should ideally be possible even if you're not using Blazor at all), wouldn't it be more applicable to have this code in the runtime library that contains the debug proxy itself?

Apologies if I'm misunderstanding the layering here!

Also alt-shift-d for chrome is not Blazor specific, so this code should also be moved.
I added here to be near to the code that does the same for chrome.

We don't have a way to get all the tabs available for debugging using a simple request as we do for chrome. We need to use the firefox remote debugging protocol to do it.

OK, up to you to decide the best place for it. If - longer term - we can factor out the non-Blazor-specific parts into a runtime package (presumably BrowserDebugProxy) that sounds ideal. Would it be worth adding an issue to track that task?

dotnet/runtime#81028

…e the beautiful message, removed the Newtonsoft from the send message, todo: remove the Newtonsoft from receive message.

SteveSandersonMS

Thanks for the updates!

@mkArtakMSFT

…ached to the blazor app (dotnet#46132) * Press alt-shift-d and open firefox debug tab attached to the blazor app * remove debugger.launch. * removing unrelated changes * Removing unnecessary changes on chrome debugging. * addressing @mkArtakMSFT comments * Addressing @mkArtakMSFT comments. * Addressing Steve comments, adding a console.warning message and remove the beautiful message, removed the Newtonsoft from the send message, todo: remove the Newtonsoft from receive message. * Completely removing newtonsoft usage as asked by steve. * Change warning message.

thaystg added 2 commits January 17, 2023 00:20

Press alt-shift-d and open firefox debug tab attached to the blazor app

839873e

remove debugger.launch.

2e88287

thaystg requested a review from a team as a code owner January 17, 2023 03:39

ghost added the area-blazor Includes: Blazor, Razor Components label Jan 17, 2023

thaystg mentioned this pull request Jan 17, 2023

[wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app dotnet/runtime#80722

Merged

thaystg added 2 commits January 17, 2023 15:16

removing unrelated changes

05aba22

Removing unnecessary changes on chrome debugging.

7c197c7

thaystg requested review from lewing and radical January 17, 2023 20:52

mkArtakMSFT reviewed Jan 19, 2023

View reviewed changes

thaystg added 2 commits January 19, 2023 16:36

addressing @mkArtakMSFT comments

6b6b2eb

Addressing @mkArtakMSFT comments.

905932b

mkArtakMSFT approved these changes Jan 19, 2023

View reviewed changes

SteveSandersonMS reviewed Jan 19, 2023

View reviewed changes

thaystg added 3 commits January 23, 2023 09:59

Addressing Steve comments, adding a console.warning message and remov…

722b020

…e the beautiful message, removed the Newtonsoft from the send message, todo: remove the Newtonsoft from receive message.

Completely removing newtonsoft usage as asked by steve.

9d3c4cd

Merge branch 'dotnet:main' into support_firefox_debugging

61daf1d

SteveSandersonMS approved these changes Jan 23, 2023

View reviewed changes

Change warning message.

47a87f6

thaystg merged commit 1a1ab56 into dotnet:main Jan 23, 2023

ghost added this to the 8.0-preview1 milestone Jan 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[blazor][wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app #46132

[blazor][wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app #46132

thaystg commented Jan 17, 2023 •

edited

Loading

mkArtakMSFT left a comment

mkArtakMSFT Jan 19, 2023

mkArtakMSFT Jan 19, 2023

mkArtakMSFT Jan 19, 2023

thaystg commented Jan 19, 2023 •

edited

Loading

mkArtakMSFT left a comment

SteveSandersonMS Jan 19, 2023

thaystg Jan 19, 2023

SteveSandersonMS Jan 19, 2023

thaystg Jan 23, 2023

SteveSandersonMS Jan 19, 2023

thaystg Jan 19, 2023

SteveSandersonMS Jan 19, 2023

thaystg Jan 23, 2023

SteveSandersonMS Jan 19, 2023

thaystg Jan 19, 2023

thaystg Jan 19, 2023

SteveSandersonMS Jan 19, 2023 •

edited

Loading

thaystg Jan 23, 2023

SteveSandersonMS left a comment

[blazor][wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app #46132

[blazor][wasm][debug]Press alt-shift-d and open firefox debug tab attached to the blazor app #46132

Conversation

thaystg commented Jan 17, 2023 • edited Loading

mkArtakMSFT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaystg commented Jan 19, 2023 • edited Loading

mkArtakMSFT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveSandersonMS Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveSandersonMS left a comment

Choose a reason for hiding this comment

thaystg commented Jan 17, 2023 •

edited

Loading

thaystg commented Jan 19, 2023 •

edited

Loading

SteveSandersonMS Jan 19, 2023 •

edited

Loading