Skip to content

script: propagate &mut JSContext in EventTarget::fire_event_with_params#44482

Merged
TimvdLippe merged 1 commit intoservo:mainfrom
elomscansio:port-EventTarget-fire_event_with_params
Apr 27, 2026
Merged

script: propagate &mut JSContext in EventTarget::fire_event_with_params#44482
TimvdLippe merged 1 commit intoservo:mainfrom
elomscansio:port-EventTarget-fire_event_with_params

Conversation

@elomscansio
Copy link
Copy Markdown
Contributor

Propagate &mut JSContext in EventTarget::fire_event_with_params and related call sites.

Testing: Successful build is enough
Fixes: Part of #42638

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 24, 2026
@elomscansio
Copy link
Copy Markdown
Contributor Author

ping @Gae24

@Gae24
Copy link
Copy Markdown
Contributor

Gae24 commented Apr 24, 2026

ping @Gae24

This PR already introduce a couple of temp_cx, passing JSContext to all Event::fire call sites will require even more effort.
That's why I suggest adding a temp_cx inside Event::dispatch_inner, the rest of the Event code can be gradually converted in the future.

Copy link
Copy Markdown
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

Seems like we are close to making this happen, exciting! Still, looks like we need at least two more preparation PRs before we can make this one stick.

Comment thread components/script/dom/html/input_element/checkbox_input_type.rs
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.

Same here, this first requires a PR to add the cx to this method.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 25, 2026
pull Bot pushed a commit to Haofei/servo that referenced this pull request Apr 25, 2026
…ervo#44498)

Propagate `&mut JSContext` in `Activatable::activation_behavior` and
related call sites.

Testing: Successful build is enough
Fixes: Part of servo#42638 

Opened to reduces the complexity of servo#44482

---------

Signed-off-by: Emmanuel Paul Elom <elomemmanuel007@gmail.com>
Signed-off-by: elomscansio <163124154+elomscansio@users.noreply.github.com>
Co-authored-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Emmanuel Paul Elom <elomemmanuel007@gmail.com>
@elomscansio elomscansio force-pushed the port-EventTarget-fire_event_with_params branch from a18ddae to c4cdf44 Compare April 26, 2026 21:05
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Apr 26, 2026
@elomscansio elomscansio requested review from Gae24 and TimvdLippe April 26, 2026 21:07
Copy link
Copy Markdown
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

This is an amazing cleanup! Thanks for sticking with us and tackling those unblocking issues we identified.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 27, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue Apr 27, 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 Apr 27, 2026
Merged via the queue into servo:main with commit 7a9931b Apr 27, 2026
33 checks passed
@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 Apr 27, 2026
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.

4 participants