servoshell: show a status bar tooltip when hovering over links

[MunishMummadi]

Description

Adds functionality to show a status bar tooltip when hovering over links in the minibrowser. This PR watches for EmbedderMsg::Status events and keeps track of the current status value in the Minibrowser. When the page has status text, an egui tooltip is displayed at the bottom left corner.

Changes Made

• Added a new field status_text: Option<String> to the Minibrowser struct to store the current status text.
• Updated Minibrowser methods:
  • queue_embedder_events_for_minibrowser_events now listens for EmbedderMsg::Status events and updates the status_text accordingly.
  • Updated update method to display a tooltip when status_text is present.
• Added a tooltip to the bottom left corner using ui.tooltip_at_rect in the TopBottomPanel show routine.

Checklist

• Created status_text field in Minibrowser struct.
• Updated queue_embedder_events_for_minibrowser_events to handle EmbedderMsg::Status.
• Added logic in update to show a tooltip when status_text is present.
• Added tooltip display in the TopBottomPanel show routine.

Related Issues

Closes #31690

[MunishMummadi]

hey @delan . Can you review my PR. Guidance is appreciated.

[eerii]

It also seems that the first commit was from #31685. If it's possible it would be great if you could rebase the changes without that commit so that there are no conflicts.

[delan]

The changes in this patch don’t match the description. For example, there is a new hovered_link field, but no status_text, and there are no changes to how we handle EmbedderMsg::Status. Have you pushed your latest changes?

sir @eerii . Am I good to go. I tried to rebase but failed. It won't repeat again. Thank you for the guidance.

In the Servo project, please avoid using honorifics like “sir” and “madam”. I understand this can be a cultural difference, but not everyone is comfortable being addressed that way, and your message would still read as polite without it. You can refer to people by name, @mention, or their pronouns, all of these are ok.

[gterzian]

The compiler reports that

Could you please post the error?

[MunishMummadi]

The compiler reports that

Could you please post the error?

follow is error I got after running ./mach build

munish@Zoro:~/servo$ ./mach build
No build type specified so assuming `--dev`.
   Compiling servoshell v0.0.1 (/home/munish/servo/ports/servoshell)
error[E0432]: unresolved import `servo::compositing::windowing::EmbedderMsg`
  --> ports/servoshell/minibrowser.rs:22:6
   |
22 |  use servo::compositing::windowing::EmbedderMsg;
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `EmbedderMsg` in `windowing`

error[E0308]: mismatched types
   --> ports/servoshell/minibrowser.rs:342:33
    |
342 |         self.status_text = Some(status);
    |                            ---- ^^^^^^ expected `String`, found `Option<String>`
    |                            |
    |                            arguments to this enum variant are incorrect
    |
    = note: expected struct `std::string::String`
                 found enum `std::option::Option<std::string::String>`
help: the type constructed contains `std::option::Option<std::string::String>` due to the type of the argument passed
   --> ports/servoshell/minibrowser.rs:342:28
    |
342 |         self.status_text = Some(status);
    |                            ^^^^^------^
    |                                 |
    |                                 this argument influences the type of `Some`
note: tuple variant defined here
   --> /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/option.rs:571:5

error[E0061]: this method takes 1 argument but 0 arguments were supplied
   --> ports/servoshell/minibrowser.rs:350:22
    |
350 |             ui.ctx().output().tooltip_text(
    |                      ^^^^^^-- an argument is missing
    |
note: method defined here
   --> /home/munish/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-0.22.0/src/context.rs:468:12
    |
468 |     pub fn output<R>(&self, reader: impl FnOnce(&PlatformOutput...
    |            ^^^^^^
help: provide the argument
    |
350 |             ui.ctx().output(/* reader */).tooltip_text(
    |                            ~~~~~~~~~~~~~~

error[E0061]: this method takes 1 argument but 0 arguments were supplied
   --> ports/servoshell/minibrowser.rs:354:62
    |
354 | ...le_width(), ui.fonts().row_height()),
    |                   ^^^^^-- an argument is missing
    |
note: method defined here
   --> /home/munish/.cargo/registry/src/index.crates.io-6f17d22bba15001f/egui-0.22.0/src/ui.rs:417:12
    |
417 |     pub fn fonts<R>(&self, reader: impl FnOnce(&Fonts) -> R) ->...
    |            ^^^^^
help: provide the argument
    |
354 |                     egui::Vec2::new(ui.available_width(), ui.fonts(/* reader */).row_height()),
    |
~~~~~~~~~~~~~~

Some errors have detailed explanations: E0061, E0308, E0432.
For more information about an error, try `rustc --explain E0061`.
error: could not compile `servoshell` (lib) due to 4 previous errors
      Timing report saved to /home/munish/servo/target/cargo-timings/cargo-timing-20240320T010049Z.html
Failed in 0:00:07
[Warning] Could not generate notification: org.freedesktop.DBus.Error.ServiceUnknown: The name org.freedesktop.Notifications was not provided bymunish@Zoro:~/servo$

[gterzian]

use servo::compositing::windowing::EmbedderMsg;

use embedder_traits::EmbedderMsg;

self.status_text = Some(status);

Did you change status_text into an option? If that is intended, I guess you have to update the type on the definition of self.

For the others, the errors are from the ui library, did you inadvertently update the version or something? Unless you changed the existing code I don't see why it would start failing.

[MunishMummadi]

Hey @gterzian I am able to get rid of the first error. I will work on the remaining things too. I need a clarification about my approach. whether my approach is in the correct direction or not. I don't expect you to look at the complete code. Thanks for the help though.

[gterzian]

I am able to get rid of the first error. I will work on the remaining things too.

Good!

whether my approach is in the correct direction or not.

With regard to the overall approach, I will refer to @delan comment above:

The changes in this patch don’t match the description. For example, there is a new hovered_link field, but no status_text, and there are no changes to how we handle EmbedderMsg::Status. Have you pushed your latest changes?

Have those comments been addressed?

[MunishMummadi]

I am able to get rid of the first error. I will work on the remaining things too.

Good!

whether my approach is in the correct direction or not.

With regard to the overall approach, I will refer to @delan comment above:

The changes in this patch don’t match the description. For example, there is a new hovered_link field, but no status_text, and there are no changes to how we handle EmbedderMsg::Status. Have you pushed your latest changes?

Have those comments been addressed?

No @gterzian I haven't pushed the latest changes. Because I am struck at what arguments to pass. Do you want me to push them.

[gterzian]

The two methods take closures as arguments.

[MunishMummadi]

The two methods take closures as arguments.

Hey @gterzian . I missed a simple thing. I updated it . Can you have look a at it.

[sandeepB3]

@MunishMummadi, before you push your code, I would suggest you to run ./mach build -d and ./mach test-tidy to ensure that building servo after your changes does not report any errors, since none of your PR pass the workflow checks, I have suggested a comment in your code which can cause ./mach build -d to report an error.

[MunishMummadi]

@MunishMummadi, before you push your code, I would suggest you to run ./mach build -d and ./mach test-tidy to ensure that building servo after your changes does not report any errors, since none of your PR pass the workflow checks, I have suggested a comment in your code which can cause ./mach build -d to report an error.

Hey @sandeepB3 . I ran ./mach fmt and ./mach test-tidy. It hasn't reported any errors. I will run ./mach build -d. thnx for the suggestion.

[sandeepB3]

I would also suggest you to run ./mach run -d -- <any link of your choice>, then test if the tooltip works when hovering over links, since even in my draft PR #31751 ./mach build -d and ./mach test-tidy does not report any error, but opening any link in the minibrowser and hovering over a link does not show the tooltip.

[MunishMummadi]

I am closing this PR as it became too messy. If possible I will link my latest updated changes in a New PR