servo
servo copied to clipboard
MiniBrowser method is_in_browser_rect returns true iff position is in toolbar
Describe the bug:
The method is_in_browser_rect on MiniBrowser returns true if (and only if) the position is inside the toolbar.
If this behavior is not a bug, perhaps the function should be renamed to something like fn is_in_browser_toolbar
.
The code (from minibrowser.rs):
/// Return true iff the given position is in the Servo browser rect.
fn is_in_browser_rect(&self, position: Point2D<f32, DeviceIndependentPixel>) -> bool {
position.y < self.toolbar_height.get()
}
It is used above in fn on_window_event
, where MouseWheel and MouseInput events are only marked as consumed if fn is_in_browser_rect returns true.
Impact is unknown to me, as I don't know how the consumed
flag effects the further handling of events; does website JavaScript receive (only) unconsumed events?
To Reproduce:
Modify fn is_in_browser_rect
to debug-print its result, and then observe its behavior by clicking around on the servo window.
Platform: Occurs on Windows 10, while using a mouse.
CC @MunishMummadi Some prior discussion occured on https://github.com/servo/servo/issues/31655, all relevant context should be here.
In the prior discussion, Munish wrote:
based on your suggestion. we can change it to something like this and by fetching the window_size in the beginning of the function and using it in
is_in_browser_rect
fn is_in_browser_rect( &self, position: Point2D<f32, DeviceIndependentPixel>, window_size: LogicalSize<f32>, ) -> bool { position.y < self.toolbar_height.get() && position.x >= 0.0 && position.x <= window_size.width }
Is my understanding correct or you are suggesting something else. even with the above change I am not able to get the functionality @Nopey
Let's not add checks for the position
's X coordinate, as I'd expect events that occur entirely outside the browser window are already correctly handled (hopefully, by not even being sent to our browser's event loop)
Switching the current code's position.y < ...
to position.y >= ...
should be sufficient.
i withdraw my conclusion. I think checking the y-coordinate should be enough.