Skip to content

devtools: Support sending js objects to firefox devtools from console.log#42296

Merged
simonwuelker merged 5 commits intoservo:mainfrom
simonwuelker:log-objects
Feb 10, 2026
Merged

devtools: Support sending js objects to firefox devtools from console.log#42296
simonwuelker merged 5 commits intoservo:mainfrom
simonwuelker:log-objects

Conversation

@simonwuelker
Copy link
Copy Markdown
Member

@simonwuelker simonwuelker commented Feb 3, 2026

This allows us to console.log objects from JS and have a preview of them appear in the console.
Interacting with said preview doesn't work yet, because the devtools object actor is a stub.

image

Testing: This change adds a test

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 3, 2026
@atbrakhi
Copy link
Copy Markdown
Member

atbrakhi commented Feb 3, 2026

@simonwuelker are you facing any issues running the Devtools test? For me doing ./mach test-devtools works. it would be nice to have new tests, but it's also important that we run it locally to make sure we are not breaking the existing tests.

We are figuring out how to put Devtools tests on CI without breaking things, until then please :)

btw added the running Devtools tests details in servo/book#202

Copy link
Copy Markdown
Member

@eerii eerii left a comment

Choose a reason for hiding this comment

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

Other than that it looks great, thanks! :)

Regarding the object actor, you are right, at the moment we aren't reusing them or keeping track of them. We are doing some work on the debugger and we are touching on that, so in the future it might be better. But that's something for later

As a heads up, @atbrakhi and I are working on the same files changing how we evaluate JavaScript, but we think it is not conflicting. We have a draft PR at #42306

Comment thread components/devtools/actors/console.rs Outdated
Comment thread components/devtools/actors/console.rs Outdated
@simonwuelker
Copy link
Copy Markdown
Member Author

@simonwuelker are you facing any issues running the Devtools test?

Running them works fine, but I'm facing some issues with testing the relevant API (:

This is my attempt at writing test case that loads a page and waits for a console.log call. It currently times out because the callback is never invoked, and I'm unsure why that is the case.

def test_console_log_arguments(self):
        self.run_servoshell(url=f"{self.base_urls[0]}/console/log.html")
        with Devtools.connect() as devtools:
            devtools.watcher.watch_resources([Resources.CONSOLE_MESSAGE])

            console = WebConsoleActor(devtools.client, devtools.targets[0]["consoleActor"])
            console_result = Future()

            async def on_console_api_call(data: dict):
                console_result.set_result(data)

            # not sure which event type is correct, so use both for now...
            devtools.client.add_event_listener(
                console.actor_id, Events.WebConsole.CONSOLE_API_CALL, on_console_api_call
            )

            devtools.client.add_event_listener(
                console.actor_id, Events.WebConsole.LOG_MESSAGE, on_console_api_call
            )

            result = console_result.result(1)
            # do something useful with the data here

(console/log.html is a test file that contains nothing but a <script> with a console.log call)

….log

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
@simonwuelker
Copy link
Copy Markdown
Member Author

simonwuelker commented Feb 5, 2026

Update: I've added a working test and it has already caught a couple bugs. I was initially scared off by the need to setup different actors for the test, but it was pretty manageable in the end!

preview = result["preview"]
self.assertEquals(preview["kind"], "Object")
self.assertEquals(preview["ownPropertiesLength"], 3)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Its likely possible to write this as a reusable assert-method for other tests but lets keep it simple for now. This is the first test for console.log.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Copy link
Copy Markdown
Member

@eerii eerii left a comment

Choose a reason for hiding this comment

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

The DevTools part looks good to me :)

I'm less familiar with the console_object_from_handle_value code so I can't comment much on that.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 9, 2026
Copy link
Copy Markdown
Member

@atbrakhi atbrakhi left a comment

Choose a reason for hiding this comment

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

Thank you for adding tests. Looks great <3

preview["ownProperties"]["baz"],
# TODO: The boolean value here should not be a string! That's a bug in our
# devtools implementation.
{"configurable": False, "enumerable": True, "value": "true", "writable": True},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would you mind filling a bug for this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I filed #42503 and marked it as a good first issue since the fix is pretty simple ^^

@atbrakhi atbrakhi added this pull request to the merge queue Feb 9, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 9, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Feb 9, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Feb 9, 2026
@simonwuelker
Copy link
Copy Markdown
Member Author

simonwuelker commented Feb 9, 2026

Interesting, I'll look into these failures later today.
I think logging an array fails now, because arrays are objects with integer keys.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Feb 10, 2026
@simonwuelker simonwuelker added this pull request to the merge queue Feb 10, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 10, 2026
Merged via the queue into servo:main with commit 60c8b8f Feb 10, 2026
33 checks passed
@simonwuelker simonwuelker deleted the log-objects branch February 10, 2026 11:19
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 10, 2026
github-merge-queue Bot pushed a commit that referenced this pull request Feb 11, 2026
In #42296 i made a mistake during the
final changes, accidentally inverting a condition. This was not caught
by CI because we don't run devtools tests in CI.

Testing: `./mach test-devtools` now passes again

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants