Skip to content

Delete per-phase dirs from /devtools-extension/#106

Open
mikehoffms wants to merge 14 commits into
MicrosoftEdge:mainfrom
mikehoffms:user/mikehoffms/devtools-extension
Open

Delete per-phase dirs from /devtools-extension/#106
mikehoffms wants to merge 14 commits into
MicrosoftEdge:mainfrom
mikehoffms:user/mikehoffms/devtools-extension

Conversation

@mikehoffms
Copy link
Copy Markdown
Contributor

@mikehoffms mikehoffms commented Oct 24, 2025

This PR is ready to merge, but don't merge this PR until merge PR 3616.

Related PRs:

Branch dir:

Changes:

  • Merged code from phase 3 dir (Memory API) & phase 4 dir (2-way messaging).
  • Moved code up from subdirs directly into a single dir.

AB#59925391

Comment thread devtools-extension/README.md Outdated
Comment thread devtools-extension/README.md Outdated
Comment thread devtools-extension/README.md Outdated
</div>

<h3>Send message from DevTools to inspected page</h3>
<input type="button" id="sayHello" value="Say hello to the inspected page"><!-- todo: alert is sometimes displayed twice (in "main" as well) -->
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alert is sometimes displayed twice, in this extension and in "main" branch.
No consistent repro steps yet; d/k why alert is displayed twice sometimes.

@captainbrosset

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested locally with the main branch (step 4), but I never saw the alert twice in a row.
If you can't get consistent repro steps, I advise resolving this comment and removing the hidden TODO for now. When/if you find consistent repro steps, please open a new issue on the Demos repo to investigate.

Suggested change
<input type="button" id="sayHello" value="Say hello to the inspected page"><!-- todo: alert is sometimes displayed twice (in "main" as well) -->
<input type="button" id="sayHello" value="Say hello to the inspected page">

Comment thread devtools-extension/devtools.js Outdated
// Send a message back to DevTools
connections[id].postMessage("Connected!");
// Send a message back to DevTools.
connections[id].postMessage("Connected!"); // todo: where can we see the message "Connected!" in DevTools?
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How/where can we see the "Connected!" message?

@captainbrosset

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent quite a lot of time investigating this without finding the answer. This message is weirdly not getting received by any listener.
I think it's either not getting sent correctly, or not getting listened to correctly.

Either way, the entire service-worker.js file is not needed by this extension. To test, I removed:

    "background": {
        "service_worker": "service-worker.js"
    },

from the manifest.json file, and all of the features of the extension (memory, click coordinates, and alert) continued to work as before.
The service worker is not playing any role in sending messages back and forth between processes in this extension.

Therefore, I think we should delete the lines above from manifest.json, and delete the entire service-worker.js file.

And then, simplify the https://review.learn.microsoft.com/en-us/microsoft-edge/extensions/developer-guide/devtools-extension?branch=pr-en-us-3616 article to reflect this.

Comment thread devtools-extension/devtools.js Outdated
</div>

<h3>Send message from DevTools to inspected page</h3>
<input type="button" id="sayHello" value="Say hello to the inspected page"><!-- todo: alert is sometimes displayed twice (in "main" as well) -->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested locally with the main branch (step 4), but I never saw the alert twice in a row.
If you can't get consistent repro steps, I advise resolving this comment and removing the hidden TODO for now. When/if you find consistent repro steps, please open a new issue on the Demos repo to investigate.

Suggested change
<input type="button" id="sayHello" value="Say hello to the inspected page"><!-- todo: alert is sometimes displayed twice (in "main" as well) -->
<input type="button" id="sayHello" value="Say hello to the inspected page">

// Send a message back to DevTools
connections[id].postMessage("Connected!");
// Send a message back to DevTools.
connections[id].postMessage("Connected!"); // todo: where can we see the message "Connected!" in DevTools?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent quite a lot of time investigating this without finding the answer. This message is weirdly not getting received by any listener.
I think it's either not getting sent correctly, or not getting listened to correctly.

Either way, the entire service-worker.js file is not needed by this extension. To test, I removed:

    "background": {
        "service_worker": "service-worker.js"
    },

from the manifest.json file, and all of the features of the extension (memory, click coordinates, and alert) continued to work as before.
The service worker is not playing any role in sending messages back and forth between processes in this extension.

Therefore, I think we should delete the lines above from manifest.json, and delete the entire service-worker.js file.

And then, simplify the https://review.learn.microsoft.com/en-us/microsoft-edge/extensions/developer-guide/devtools-extension?branch=pr-en-us-3616 article to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants