diff --git a/Cargo.lock b/Cargo.lock index f1c707a4cd7..2da98bf5e3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1624,6 +1624,7 @@ dependencies = [ "http 1.3.1", "hyper_serde", "ipc-channel", + "keyboard-types", "log", "malloc_size_of_derive", "net_traits", diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 5a8bc4c0b08..888d18ef65b 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1985,6 +1985,20 @@ where warn!("No webdriver_input_command_reponse_sender"); } }, + ScriptToConstellationMessage::ForwardKeyboardScroll(pipeline_id, scroll) => { + if let Some(pipeline) = self.pipelines.get(&pipeline_id) { + if let Err(error) = + pipeline + .event_loop + .send(ScriptThreadMessage::ForwardKeyboardScroll( + pipeline_id, + scroll, + )) + { + warn!("Could not forward {scroll:?} to {pipeline_id}: {error:?}"); + } + } + }, } } diff --git a/components/constellation/tracing.rs b/components/constellation/tracing.rs index 3cb28ad5af9..28f6632e89a 100644 --- a/components/constellation/tracing.rs +++ b/components/constellation/tracing.rs @@ -182,6 +182,7 @@ mod from_script { Self::ReportMemory(..) => target!("ReportMemory"), Self::WebDriverInputComplete(..) => target!("WebDriverInputComplete"), Self::FinishJavaScriptEvaluation(..) => target!("FinishJavaScriptEvaluation"), + Self::ForwardKeyboardScroll(..) => target!("ForwardKeyboardScroll"), } } } diff --git a/components/layout/flow/root.rs b/components/layout/flow/root.rs index 50e07a1f565..9330d85836b 100644 --- a/components/layout/flow/root.rs +++ b/components/layout/flow/root.rs @@ -42,7 +42,7 @@ pub struct BoxTree { root: BlockFormattingContext, /// Whether or not the viewport should be sensitive to scrolling input events in two axes - viewport_scroll_sensitivity: AxesScrollSensitivity, + pub(crate) viewport_overflow: AxesOverflow, } impl BoxTree { @@ -65,10 +65,7 @@ impl BoxTree { // From https://www.w3.org/TR/css-overflow-3/#overflow-propagation: // > If visible is applied to the viewport, it must be interpreted as auto. // > If clip is applied to the viewport, it must be interpreted as hidden. - viewport_scroll_sensitivity: AxesScrollSensitivity { - x: viewport_overflow.x.to_scrollable().into(), - y: viewport_overflow.y.to_scrollable().into(), - }, + viewport_overflow: viewport_overflow.to_scrollable(), } } @@ -266,11 +263,16 @@ impl BoxTree { &mut root_fragments, ); + let viewport_scroll_sensitivity = AxesScrollSensitivity { + x: self.viewport_overflow.x.into(), + y: self.viewport_overflow.y.into(), + }; + FragmentTree::new( layout_context, root_fragments, physical_containing_block, - self.viewport_scroll_sensitivity, + viewport_scroll_sensitivity, ) } } diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index f83470c4155..671e12e5022 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -333,11 +333,17 @@ impl Layout for LayoutThread { #[servo_tracing::instrument(skip_all)] fn query_scroll_container( &self, - node: TrustedNodeAddress, + node: Option, flags: ScrollContainerQueryFlags, ) -> Option { - let node = unsafe { ServoLayoutNode::new(&node) }; - process_scroll_container_query(node, flags) + let node = unsafe { node.as_ref().map(|node| ServoLayoutNode::new(node)) }; + let viewport_overflow = self + .box_tree + .borrow() + .as_ref() + .expect("Should have a BoxTree for all scroll container queries.") + .viewport_overflow; + process_scroll_container_query(node, flags, viewport_overflow) } #[servo_tracing::instrument(skip_all)] diff --git a/components/layout/query.rs b/components/layout/query.rs index e73344a7ad8..83d96a8c421 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -12,7 +12,7 @@ use euclid::{SideOffsets2D, Size2D}; use itertools::Itertools; use layout_api::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; use layout_api::{ - BoxAreaType, LayoutElementType, LayoutNodeType, OffsetParentResponse, + AxesOverflow, BoxAreaType, LayoutElementType, LayoutNodeType, OffsetParentResponse, ScrollContainerQueryFlags, ScrollContainerResponse, }; use script::layout_dom::{ServoLayoutNode, ServoThreadSafeLayoutNode}; @@ -676,9 +676,14 @@ pub fn process_offset_parent_query( /// #[inline] pub(crate) fn process_scroll_container_query( - node: ServoLayoutNode<'_>, + node: Option>, query_flags: ScrollContainerQueryFlags, + viewport_overflow: AxesOverflow, ) -> Option { + let Some(node) = node else { + return Some(ScrollContainerResponse::Viewport(viewport_overflow)); + }; + let layout_data = node.to_threadsafe().inner_layout_data()?; // 1. If any of the following holds true, return null and terminate this algorithm: @@ -776,7 +781,7 @@ pub(crate) fn process_scroll_container_query( match current_position_value { Position::Fixed => None, - _ => Some(ScrollContainerResponse::Viewport), + _ => Some(ScrollContainerResponse::Viewport(viewport_overflow)), } } diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 2ad54e623b3..a4024505432 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -933,6 +933,7 @@ malloc_size_of_is_stylo_malloc_size_of!(style::values::computed::FontWeight); malloc_size_of_is_stylo_malloc_size_of!(style::values::computed::font::SingleFontFamily); malloc_size_of_is_stylo_malloc_size_of!(style::values::computed::JustifyContent); malloc_size_of_is_stylo_malloc_size_of!(style::values::specified::align::AlignFlags); +malloc_size_of_is_stylo_malloc_size_of!(style::values::specified::box_::Overflow); malloc_size_of_is_stylo_malloc_size_of!(style::values::specified::TextDecorationLine); malloc_size_of_is_stylo_malloc_size_of!(stylo_dom::ElementState); diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index bf5ce902a31..f96d70a538b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -33,7 +33,10 @@ use euclid::default::{Rect, Size2D}; use html5ever::{LocalName, Namespace, QualName, local_name, ns}; use hyper_serde::Serde; use js::rust::{HandleObject, HandleValue, MutableHandleValue}; -use layout_api::{PendingRestyle, ReflowGoal, ReflowPhasesRun, RestyleReason, TrustedNodeAddress}; +use layout_api::{ + PendingRestyle, ReflowGoal, ReflowPhasesRun, RestyleReason, ScrollContainerQueryFlags, + TrustedNodeAddress, +}; use metrics::{InteractiveFlag, InteractiveWindow, ProgressiveWebMetrics}; use net_traits::CookieSource::NonHTTP; use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl}; @@ -173,7 +176,7 @@ use crate::dom::processinginstruction::ProcessingInstruction; use crate::dom::promise::Promise; use crate::dom::range::Range; use crate::dom::resizeobserver::{ResizeObservationDepth, ResizeObserver}; -use crate::dom::scrolling_box::{ScrollingBox, ScrollingBoxSource}; +use crate::dom::scrolling_box::ScrollingBox; use crate::dom::selection::Selection; use crate::dom::servoparser::ServoParser; use crate::dom::shadowroot::ShadowRoot; @@ -4447,8 +4450,10 @@ impl Document { self.active_sandboxing_flag_set.set(flags) } - pub(crate) fn viewport_scrolling_box(&self) -> ScrollingBox { - ScrollingBox::new(ScrollingBoxSource::Viewport(DomRoot::from_ref(self))) + pub(crate) fn viewport_scrolling_box(&self, flags: ScrollContainerQueryFlags) -> ScrollingBox { + self.window() + .scrolling_box_query(None, flags) + .expect("We should always have a ScrollingBox for the Viewport") } } diff --git a/components/script/dom/document_event_handler.rs b/components/script/dom/document_event_handler.rs index f87b58f78bb..21c18ddead5 100644 --- a/components/script/dom/document_event_handler.rs +++ b/components/script/dom/document_event_handler.rs @@ -10,7 +10,7 @@ use std::rc::Rc; use std::time::{Duration, Instant}; use base::generic_channel; -use constellation_traits::ScriptToConstellationMessage; +use constellation_traits::{KeyboardScroll, ScriptToConstellationMessage}; use embedder_traits::{ Cursor, EditingActionEvent, EmbedderMsg, GamepadEvent as EmbedderGamepadEvent, GamepadSupportedHapticEffects, GamepadUpdateType, ImeEvent, InputEvent, @@ -20,6 +20,7 @@ use embedder_traits::{ }; use euclid::{Point2D, Vector2D}; use ipc_channel::ipc; +use js::jsapi::JSAutoRealm; use keyboard_types::{Code, Key, KeyState, Modifiers, NamedKey}; use layout_api::{ScrollContainerQueryFlags, node_id_from_scroll_id}; use script_bindings::codegen::GenericBindings::DocumentBinding::DocumentMethods; @@ -31,6 +32,7 @@ use script_bindings::codegen::GenericBindings::TouchBinding::TouchMethods; use script_bindings::codegen::GenericBindings::WindowBinding::{ScrollBehavior, WindowMethods}; use script_bindings::inheritance::Castable; use script_bindings::num::Finite; +use script_bindings::reflector::DomObject; use script_bindings::root::{Dom, DomRoot, DomSlice}; use script_bindings::script_runtime::CanGc; use script_bindings::str::DOMString; @@ -48,7 +50,7 @@ use crate::dom::event::{EventBubbles, EventCancelable, EventComposed, EventDefau use crate::dom::gamepad::gamepad::{Gamepad, contains_user_gesture}; use crate::dom::gamepad::gamepadevent::GamepadEventType; use crate::dom::inputevent::HitTestResult; -use crate::dom::node::{self, Node, ShadowIncluding}; +use crate::dom::node::{self, Node, NodeTraits, ShadowIncluding}; use crate::dom::pointerevent::PointerId; use crate::dom::scrolling_box::ScrollingBoxAxis; use crate::dom::types::{ @@ -1486,60 +1488,94 @@ impl DocumentEventHandler { if !event.modifiers().is_empty() { return; } - - let scroll_axis = match event.key() { - Key::Named( - NamedKey::Home | - NamedKey::End | - NamedKey::PageDown | - NamedKey::PageUp | - NamedKey::ArrowUp | - NamedKey::ArrowDown, - ) => ScrollingBoxAxis::Y, - Key::Named(NamedKey::ArrowLeft | NamedKey::ArrowRight) => ScrollingBoxAxis::X, + let scroll = match event.key() { + Key::Named(NamedKey::ArrowDown) => KeyboardScroll::Down, + Key::Named(NamedKey::ArrowLeft) => KeyboardScroll::Left, + Key::Named(NamedKey::ArrowRight) => KeyboardScroll::Right, + Key::Named(NamedKey::ArrowUp) => KeyboardScroll::Up, + Key::Named(NamedKey::End) => KeyboardScroll::End, + Key::Named(NamedKey::Home) => KeyboardScroll::Home, + Key::Named(NamedKey::PageDown) => KeyboardScroll::PageDown, + Key::Named(NamedKey::PageUp) => KeyboardScroll::PageUp, _ => return, }; + self.do_keyboard_scroll(scroll); + } + + pub(crate) fn do_keyboard_scroll(&self, scroll: KeyboardScroll) { + let scroll_axis = match scroll { + KeyboardScroll::Left | KeyboardScroll::Right => ScrollingBoxAxis::X, + _ => ScrollingBoxAxis::Y, + }; let document = self.window.Document(); let mut scrolling_box = document .get_focused_element() .or(self.most_recently_clicked_element.get()) .and_then(|element| element.scrolling_box(ScrollContainerQueryFlags::Inclusive)) - .unwrap_or_else(|| document.viewport_scrolling_box()); + .unwrap_or_else(|| { + document.viewport_scrolling_box(ScrollContainerQueryFlags::Inclusive) + }); while !scrolling_box.can_keyboard_scroll_in_axis(scroll_axis) { // Always fall back to trying to scroll the entire document. if scrolling_box.is_viewport() { break; } - let parent = scrolling_box - .parent() - .unwrap_or_else(|| document.viewport_scrolling_box()); + let parent = scrolling_box.parent().unwrap_or_else(|| { + document.viewport_scrolling_box(ScrollContainerQueryFlags::Inclusive) + }); scrolling_box = parent; } + // If this is the viewport and we cannot scroll, try to ask a parent viewport to scroll, + // if we are inside an ` + + + + + + + + + + + +
+ Lorem ipsum dolor sit amet, +
+ + diff --git a/tests/wpt/mozilla/tests/css/keyboard-scrolling.html b/tests/wpt/mozilla/tests/mozilla/keyboard-scrolling.html similarity index 80% rename from tests/wpt/mozilla/tests/css/keyboard-scrolling.html rename to tests/wpt/mozilla/tests/mozilla/keyboard-scrolling.html index 2d9a0c40272..45916c5d11f 100644 --- a/tests/wpt/mozilla/tests/css/keyboard-scrolling.html +++ b/tests/wpt/mozilla/tests/mozilla/keyboard-scrolling.html @@ -1,6 +1,6 @@ -CSS test: Calc expressions with numbers should still serialize as calc() +A test to verify that keyboard scrolling works properly in Servo. @@ -36,15 +36,8 @@
Lorem ipsum dolor sit amet,
- - - -
+
Lorem ipsum dolor sit amet,
@@ -66,13 +59,9 @@ const pageSize = scrollportHeight => scrollportHeight - 2 * lineSize; const pressKeyAndAssert = async (key, element, [expectedX, expectedY], description) => { await test_driver.send_keys(document.body, key); const actualX = - element == null ? scrollX - : element.nodeName == "IFRAME" ? element.contentWindow.scrollX - : element.scrollLeft; + element == null ? scrollX : element.scrollLeft; const actualY = - element == null ? scrollY - : element.nodeName == "IFRAME" ? element.contentWindow.scrollY - : element.scrollTop; + element == null ? scrollY : element.scrollTop; assert_array_equals([actualX, actualY], [expectedX, expectedY], description); }; @@ -156,27 +145,6 @@ promise_test(async () => { await pressKeyAndAssert(home, null, [0, 0], "Home key scrolls viewport to top"); }, "Keyboard scrolling chains past inactive overflow:scroll DIVs"); -promise_test(async () => { - await test_driver.click(iframe); - - await pressKeyAndAssert(end, iframe, [0, iframe.contentDocument.documentElement.scrollHeight - iframe.contentWindow.innerHeight], "End key scrolls #iframe to bottom"); - await pressKeyAndAssert(home, iframe, [0, 0], "Home key scrolls #iframe to top"); - await pressKeyAndAssert(arrowDown, iframe, [0, lineSize], "ArrowDown key scrolls #iframe down by a line"); - await pressKeyAndAssert(arrowDown, iframe, [0, lineSize * 2], "ArrowDown key scrolls #iframe down by a line"); - await pressKeyAndAssert(arrowUp, iframe, [0, lineSize], "ArrowUp key scrolls #iframe up by a line"); - await pressKeyAndAssert(arrowUp, iframe, [0, 0], "ArrowUp key scrolls #iframe up by a line"); - await pressKeyAndAssert(arrowRight, iframe, [lineSize, 0], "ArrowRight key scrolls #iframe right by a line"); - await pressKeyAndAssert(arrowRight, iframe, [lineSize * 2, 0], "ArrowRight key scrolls #iframe right by a line"); - await pressKeyAndAssert(arrowLeft, iframe, [lineSize, 0], "ArrowLeft key scrolls #iframe left by a line"); - await pressKeyAndAssert(arrowLeft, iframe, [0, 0], "ArrowLeft key scrolls #iframe left by a line"); - await pressKeyAndAssert(pageDown, iframe, [0, pageSize(iframe.contentWindow.innerHeight)], "PageDown key scrolls #iframe down by almost a screenful"); - await pressKeyAndAssert(pageDown, iframe, [0, pageSize(iframe.contentWindow.innerHeight) * 2], "PageDown key scrolls #iframe down by almost a screenful"); - await pressKeyAndAssert(pageUp, iframe, [0, pageSize(iframe.contentWindow.innerHeight)], "PageUp key scrolls #iframe up by almost a screenful"); - await pressKeyAndAssert(pageUp, iframe, [0, 0], "PageUp key scrolls #iframe up by almost a screenful"); - - // TODO: test that scrolls chain up from iframe when they fail. -}, "Keyboard scrolling works in #iframe"); - promise_test(async () => { await test_driver.click(boxWithOverflowHidden);