Skip to content

Conversation

@arihant2math
Copy link
Contributor

@arihant2math arihant2math commented Sep 13, 2025

Adds a righclick menu on desktop servoshell. At the moment, the rightclick menu displays completely disabled options when clicking on a link. Additionally custom rightclick menus aren't supported (I still need to clean up the API a bit).

Supports: windows and MacOS.

Testing: Manual
Fixes: Partially #38062

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 13, 2025
@servo-highfive
Copy link

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/document_event_handler.rs
  • @asajeffrey: components/script/dom/document_event_handler.rs

@servo-highfive
Copy link

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
  • These commits modify unsafe code. Please review it carefully!

@arihant2math arihant2math added S-work-in-progress and removed S-awaiting-review There is new code that needs to be reviewed. labels Sep 13, 2025
Signed-off-by: Ashwin Naren <arihant2math@gmail.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 13, 2025
Comment on lines +519 to 533
fn show_native_context_menu(
&self,
_webview: WebView,
result_sender: GenericSender<ContextMenuResult>,
_: Option<String>,
_: Vec<String>,
_options: NativeContextMenu,
) {
let _ = result_sender.send(ContextMenuResult::Ignored);
}

fn show_custom_context_menu(
&self,
_webview: WebView,
result_sender: GenericSender<ContextMenuResult>,
_items: Vec<ContextMenuItem>,
) {
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between a native and custom context menu?

Copy link
Contributor Author

@arihant2math arihant2math Sep 20, 2025

Choose a reason for hiding this comment

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

Native menu is the one the browser will display by default, with actions that can only be handled by the browser itself. Custom context menus are created by the script thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, I'm considering removing that functionality.

),
/// Show a context menu to the user
ShowContextMenu(
ShowCustomContextMenu(
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be unused?

libservo = { path = "../../components/servo", features = ["background_hang_monitor", "bluetooth", "testbinding"] }
log = { workspace = true }
mime_guess = { workspace = true }
muda = "0.17.1"
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not to add another dependency, especially one that adds dependencies on platform-specific toolkits. Please implement these menus as egui context menus.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Sep 16, 2025
@mrobinson
Copy link
Member

This change has been superseded by recent work improving the Servo context menu APIs.

@mrobinson mrobinson closed this Nov 13, 2025
@arihant2math arihant2math deleted the right-click branch November 13, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-needs-rebase There are merge conflict errors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants