diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index eab280ac85b..43fa3f5a132 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -651,18 +651,30 @@ impl LayoutThread { ); } + /// Checks whether we need to update the scroll node, and report whether the + /// node is scrolled. We need to update the scroll node whenever it is requested. + fn handle_update_scroll_node_request(&self, reflow_request: &ReflowRequest) -> bool { + if let ReflowGoal::UpdateScrollNode(external_scroll_id, offset) = reflow_request.reflow_goal + { + self.set_scroll_offset_from_script(external_scroll_id, offset) + } else { + false + } + } + /// The high-level routine that performs layout. #[servo_tracing::instrument(skip_all)] fn handle_reflow(&mut self, mut reflow_request: ReflowRequest) -> Option { self.maybe_print_reflow_event(&reflow_request); if self.can_skip_reflow_request_entirely(&reflow_request) { - if let ReflowGoal::UpdateScrollNode(external_scroll_id, offset) = - reflow_request.reflow_goal - { - self.set_scroll_offset_from_script(external_scroll_id, offset); - } - return None; + // We could skip the layout, but we might need to update the scroll node. + let update_scroll_reflow_target_scrolled = + self.handle_update_scroll_node_request(&reflow_request); + + return Some(ReflowResult::new_without_relayout( + update_scroll_reflow_target_scrolled, + )); } let document = unsafe { ServoLayoutNode::new(&reflow_request.document) }; @@ -692,10 +704,8 @@ impl LayoutThread { self.build_stacking_context_tree(&reflow_request, damage); let built_display_list = self.build_display_list(&reflow_request, damage, &image_resolver); - if let ReflowGoal::UpdateScrollNode(external_scroll_id, offset) = reflow_request.reflow_goal - { - self.set_scroll_offset_from_script(external_scroll_id, offset); - } + let update_scroll_reflow_target_scrolled = + self.handle_update_scroll_node_request(&reflow_request); let pending_images = std::mem::take(&mut *image_resolver.pending_images.lock()); let pending_rasterization_images = @@ -705,7 +715,9 @@ impl LayoutThread { built_display_list, pending_images, pending_rasterization_images, - iframe_sizes, + iframe_sizes: Some(iframe_sizes), + update_scroll_reflow_target_scrolled, + processed_relayout: true, }) } @@ -1093,10 +1105,10 @@ impl LayoutThread { &self, external_scroll_id: ExternalScrollId, offset: LayoutVector2D, - ) { + ) -> bool { let mut stacking_context_tree = self.stacking_context_tree.borrow_mut(); let Some(stacking_context_tree) = stacking_context_tree.as_mut() else { - return; + return false; }; if let Some(offset) = stacking_context_tree @@ -1114,6 +1126,9 @@ impl LayoutThread { offset, external_scroll_id, ); + true + } else { + false } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index c055240fecc..60694375529 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2413,24 +2413,23 @@ impl Document { // Step 2 // > For each item target in doc’s pending scroll event targets, in the order they // > were added to the list, run these substeps: - for target in self.pending_scroll_event_targets.borrow().iter() { - // Step 2.1 - // > If target is a Document, fire an event named scroll that bubbles at target. + // Step 3. + // > Empty doc’s pending scroll event targets. + // Since the scroll event callback could trigger another scroll event, we are taking all of the + // current scroll event to avoid borrow checking error. + rooted_vec!(let notify_list <- self.pending_scroll_event_targets.take().into_iter()); + for target in notify_list.iter() { if target.downcast::().is_some() { + // Step 2.1 + // > If target is a Document, fire an event named scroll that bubbles at target. target.fire_bubbling_event(Atom::from("scroll"), can_gc); - } - - // Step 2.2 - // > Otherwise, fire an event named scroll at target. - if target.downcast::().is_some() { + } else if target.downcast::().is_some() { + // Step 2.2 + // > Otherwise, fire an event named scroll at target. target.fire_event(Atom::from("scroll"), can_gc); } } - // Step 3. - // > Empty doc’s pending scroll event targets. - self.pending_scroll_event_targets.borrow_mut().clear(); - // Step 4. // > Run the steps to dispatch pending scrollsnapchange events for doc. // TODO(#7673): Implement scroll snapping @@ -3700,7 +3699,9 @@ impl Document { receiver.recv().unwrap(); } - self.window().reflow(ReflowGoal::UpdateTheRendering, can_gc) + self.window() + .reflow(ReflowGoal::UpdateTheRendering, can_gc) + .reflow_issued } pub(crate) fn id_map(&self) -> Ref>>> { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index e060ada1d6a..00d1e0a62c6 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -2476,6 +2476,7 @@ impl Element { } // https://drafts.csswg.org/cssom-view/#dom-element-scroll + // TODO(stevennovaryo): Need to update the scroll API to follow the spec since it is quite outdated. pub(crate) fn scroll(&self, x_: f64, y_: f64, behavior: ScrollBehavior, can_gc: CanGc) { // Step 1.2 or 2.3 let x = if x_.is_finite() { x_ } else { 0.0f64 }; @@ -2524,7 +2525,7 @@ impl Element { } // Step 11 - win.scroll_node(node, x, y, behavior, can_gc); + win.scroll_an_element(self, x, y, behavior, can_gc); } /// @@ -3180,6 +3181,7 @@ impl ElementMethods for Element { } // https://drafts.csswg.org/cssom-view/#dom-element-scrolltop + // TODO(stevennovaryo): Need to update the scroll API to follow the spec since it is quite outdated. fn SetScrollTop(&self, y_: f64, can_gc: CanGc) { let behavior = ScrollBehavior::Auto; @@ -3229,7 +3231,7 @@ impl ElementMethods for Element { } // Step 11 - win.scroll_node(node, self.ScrollLeft(can_gc), y, behavior, can_gc); + win.scroll_an_element(self, self.ScrollLeft(can_gc), y, behavior, can_gc); } // https://drafts.csswg.org/cssom-view/#dom-element-scrolltop @@ -3329,7 +3331,7 @@ impl ElementMethods for Element { } // Step 11 - win.scroll_node(node, x, self.ScrollTop(can_gc), behavior, can_gc); + win.scroll_an_element(self, x, self.ScrollTop(can_gc), behavior, can_gc); } // https://drafts.csswg.org/cssom-view/#dom-element-scrollwidth diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index e60a5e272b8..17bf449d0ef 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -225,6 +225,25 @@ impl LayoutBlocker { type PendingImageRasterizationKey = (PendingImageId, DeviceIntSize); +/// Feedbacks of the reflow that is required by the one who is initiating the reflow. +pub(crate) struct WindowReflowResult { + /// Whether the reflow actually happened and it sends a new display list to the embedder. + pub reflow_issued: bool, + /// Whether the reflow is for [ReflowGoal::UpdateScrollNode] and the target is scrolled. + /// Specifically, a node is scrolled whenever the scroll position of it changes. Note + /// that reflow that is cancalled would not scroll the target. + pub update_scroll_reflow_target_scrolled: bool, +} + +impl WindowReflowResult { + fn new_empty() -> Self { + WindowReflowResult { + reflow_issued: false, + update_scroll_reflow_target_scrolled: false, + } + } +} + #[dom_struct] pub(crate) struct Window { globalscope: GlobalScope, @@ -2145,16 +2164,30 @@ impl Window { y: f32, scroll_id: ExternalScrollId, _behavior: ScrollBehavior, - _element: Option<&Element>, + element: Option<&Element>, can_gc: CanGc, ) { // TODO Step 1 // TODO(mrobinson, #18709): Add smooth scrolling support to WebRender so that we can // properly process ScrollBehavior here. - self.reflow( + let WindowReflowResult { + update_scroll_reflow_target_scrolled, + .. + } = self.reflow( ReflowGoal::UpdateScrollNode(scroll_id, Vector2D::new(x, y)), can_gc, ); + + // > If the scroll position did not change as a result of the user interaction or programmatic + // > invocation, where no translations were applied as a result, then no scrollend event fires + // > because no scrolling occurred. + // Even though the note mention the scrollend, it is relevant to the scroll as well. + if update_scroll_reflow_target_scrolled { + match element { + Some(el) => self.Document().handle_element_scroll_event(el), + None => self.Document().handle_viewport_scroll_event(), + }; + } } pub(crate) fn device_pixel_ratio(&self) -> Scale { @@ -2183,11 +2216,9 @@ impl Window { /// no reflow is performed. If reflow is suppressed, no reflow will be performed for ForDisplay /// goals. /// - /// Returns true if layout actually happened and it sent a new display list to the renderer. - /// /// NOTE: This method should almost never be called directly! Layout and rendering updates should /// happen as part of the HTML event loop via *update the rendering*. - fn force_reflow(&self, reflow_goal: ReflowGoal) -> bool { + fn force_reflow(&self, reflow_goal: ReflowGoal) -> WindowReflowResult { let document = self.Document(); document.ensure_safe_to_run_script_or_layout(); @@ -2199,7 +2230,7 @@ impl Window { self.layout_blocker.get().layout_blocked() { debug!("Suppressing pre-load-event reflow pipeline {pipeline_id}"); - return false; + return WindowReflowResult::new_empty(); } debug!("script: performing reflow for goal {reflow_goal:?}"); @@ -2251,25 +2282,36 @@ impl Window { }; let Some(results) = self.layout.borrow_mut().reflow(reflow) else { - return false; + return WindowReflowResult::new_empty(); }; - debug!("script: layout complete"); - if let Some(marker) = marker { - self.emit_timeline_marker(marker.end()); + // We are maintaining the previous behavior of layout where we are skipping these behavior if we are not + // doing layout calculation. + if results.processed_relayout { + debug!("script: layout complete"); + if let Some(marker) = marker { + self.emit_timeline_marker(marker.end()); + } + + self.handle_pending_images_post_reflow( + results.pending_images, + results.pending_rasterization_images, + ); + if let Some(iframe_sizes) = results.iframe_sizes { + document + .iframes_mut() + .handle_new_iframe_sizes_after_layout(self, iframe_sizes); + } + document.update_animations_post_reflow(); + self.update_constellation_epoch(); + } else { + debug!("script: layout-side reflow finished without relayout"); } - self.handle_pending_images_post_reflow( - results.pending_images, - results.pending_rasterization_images, - ); - document - .iframes_mut() - .handle_new_iframe_sizes_after_layout(self, results.iframe_sizes); - document.update_animations_post_reflow(); - self.update_constellation_epoch(); - - results.built_display_list + WindowReflowResult { + reflow_issued: results.built_display_list, + update_scroll_reflow_target_scrolled: results.update_scroll_reflow_target_scrolled, + } } /// Reflows the page if it's possible to do so and the page is dirty. Returns true if layout @@ -2278,10 +2320,10 @@ impl Window { /// NOTE: This method should almost never be called directly! Layout and rendering updates /// should happen as part of the HTML event loop via *update the rendering*. Currerntly, the /// only exceptions are script queries and scroll requests. - pub(crate) fn reflow(&self, reflow_goal: ReflowGoal, can_gc: CanGc) -> bool { + pub(crate) fn reflow(&self, reflow_goal: ReflowGoal, can_gc: CanGc) -> WindowReflowResult { // Never reflow inactive Documents. if !self.Document().is_fully_active() { - return false; + return WindowReflowResult::new_empty(); } // Count the pending web fonts before layout, in case a font loads during the layout. @@ -2290,7 +2332,7 @@ impl Window { self.Document().ensure_safe_to_run_script_or_layout(); let updating_the_rendering = reflow_goal == ReflowGoal::UpdateTheRendering; - let issued_reflow = self.force_reflow(reflow_goal); + let reflow_result = self.force_reflow(reflow_goal); let document = self.Document(); let font_face_set = document.Fonts(can_gc); @@ -2348,7 +2390,7 @@ impl Window { } } - issued_reflow + reflow_result } /// If parsing has taken a long time and reflows are still waiting for the `load` event, @@ -2428,8 +2470,9 @@ impl Window { let _ = receiver.recv(); } - pub(crate) fn layout_reflow(&self, query_msg: QueryMsg, can_gc: CanGc) -> bool { - self.reflow(ReflowGoal::LayoutQuery(query_msg), can_gc) + /// Trigger a reflow that is required by a certain queries. + pub(crate) fn layout_reflow(&self, query_msg: QueryMsg, can_gc: CanGc) { + self.reflow(ReflowGoal::LayoutQuery(query_msg), can_gc); } pub(crate) fn resolved_font_style_query( @@ -2507,33 +2550,46 @@ impl Window { can_gc: CanGc, ) -> Vector2D { self.layout_reflow(QueryMsg::ScrollingAreaOrOffsetQuery, can_gc); + self.scroll_offset_query_with_external_scroll_id_no_reflow(external_scroll_id) + } + + fn scroll_offset_query_with_external_scroll_id_no_reflow( + &self, + external_scroll_id: ExternalScrollId, + ) -> Vector2D { self.layout .borrow() .scroll_offset(external_scroll_id) .unwrap_or_default() } - // https://drafts.csswg.org/cssom-view/#element-scrolling-members - pub(crate) fn scroll_node( + /// + // TODO(stevennovaryo): Need to update the scroll API to follow the spec since it is quite outdated. + pub(crate) fn scroll_an_element( &self, - node: &Node, + element: &Element, x_: f64, y_: f64, behavior: ScrollBehavior, can_gc: CanGc, ) { let scroll_id = ExternalScrollId( - combine_id_with_fragment_type(node.to_opaque().id(), FragmentType::FragmentBody), + combine_id_with_fragment_type( + element.upcast::().to_opaque().id(), + FragmentType::FragmentBody, + ), self.pipeline_id().into(), ); - // Step 12 + // Step 6. + // > Perform a scroll of box to position, element as the associated element and behavior as + // > the scroll behavior. self.perform_a_scroll( x_.to_f32().unwrap_or(0.0f32), y_.to_f32().unwrap_or(0.0f32), scroll_id, behavior, - None, + Some(element), can_gc, ); } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index f291b23c89d..28d44487f13 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1295,8 +1295,14 @@ impl ScriptThread { self.process_pending_input_events(*pipeline_id, can_gc); // > 8. For each doc of docs, run the resize steps for doc. [CSSOMVIEW] - if document.window().run_the_resize_steps(can_gc) { - // Evaluate media queries and report changes. + let resized = document.window().run_the_resize_steps(can_gc); + + // > 9. For each doc of docs, run the scroll steps for doc. + document.run_the_scroll_steps(can_gc); + + // Media queries is only relevant when there are resizing. + if resized { + // 10. For each doc of docs, evaluate media queries and report changes for doc. document .window() .evaluate_media_queries_and_report_changes(can_gc); @@ -1306,9 +1312,6 @@ impl ScriptThread { document.react_to_environment_changes() } - // > 9. For each doc of docs, run the scroll steps for doc. - document.run_the_scroll_steps(can_gc); - // > 11. For each doc of docs, update animations and send events for doc, passing // > in relative high resolution time given frameTimestamp and doc's relevant // > global object as the timestamp [WEBANIMATIONS] diff --git a/components/shared/layout/lib.rs b/components/shared/layout/lib.rs index 8859875e2d7..7d182fe5fc4 100644 --- a/components/shared/layout/lib.rs +++ b/components/shared/layout/lib.rs @@ -344,7 +344,8 @@ pub enum ReflowGoal { LayoutQuery(QueryMsg), /// Tells layout about a single new scrolling offset from the script. The rest will - /// remain untouched and layout won't forward this back to script. + /// remain untouched. Layout will forward whether the element is scrolled through + /// [ReflowResult]. UpdateScrollNode(ExternalScrollId, LayoutVector2D), } @@ -391,8 +392,28 @@ pub struct ReflowResult { pub pending_rasterization_images: Vec, /// The list of iframes in this layout and their sizes, used in order /// to communicate them with the Constellation and also the `Window` - /// element of their content pages. - pub iframe_sizes: IFrameSizes, + /// element of their content pages. Returning None if incremental reflow + /// finished before reaching this stage of the layout. I.e., no update + /// required. + pub iframe_sizes: Option, + /// Whether the reflow is for [ReflowGoal::UpdateScrollNode] and the target is scrolled. + /// Specifically, a node is scrolled whenever the scroll position of it changes. + pub update_scroll_reflow_target_scrolled: bool, + /// Do the reflow results in a new component within layout. Incremental layout could be + /// skipped if it is deemed unnecessary or the required component is not ready to be + /// processed. + pub processed_relayout: bool, +} + +impl ReflowResult { + /// In incremental reflow, we could skip the layout calculation completely, if it is deemed + /// unecessary. In those cases, many of the [ReflowResult] would be irrelevant. + pub fn new_without_relayout(update_scroll_reflow_target_scrolled: bool) -> Self { + ReflowResult { + update_scroll_reflow_target_scrolled, + ..Default::default() + } + } } /// Information needed for a script-initiated reflow that requires a restyle diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 38d9c3169c5..3dc39e93a10 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -467655,6 +467655,10 @@ "5df4fa279363798f2f471467ed674c092c7a1749", [] ], + "large-dimension-document.sub.html": [ + "5d29f7be0bbb77e6303598aef833a810e84b6e80", + [] + ], "prefixed-animation-event-tests.js": [ "021b6bb9dfdc422d1a6c3c9c4a1a039f89a901d5", [] @@ -633745,6 +633749,20 @@ } ] ], + "scroll-event-fired-to-element.html": [ + "3484a2ec92b2bd8faa1d5147568f13a83b8c59d9", + [ + null, + {} + ] + ], + "scroll-event-fired-to-iframe.html": [ + "2ff3de79d33fa0a676a0eda4f109b6d0e634cc9d", + [ + null, + {} + ] + ], "scrollend-event-fired-after-sequence-of-scrolls.tentative.html": [ "dab6dcc9bd8d67a514ca95daf3e1d64b10a45188", [ diff --git a/tests/wpt/meta/html/webappapis/scripting/event-loops/new-scroll-event-dispatched-at-next-updating-rendering-time.html.ini b/tests/wpt/meta/html/webappapis/scripting/event-loops/new-scroll-event-dispatched-at-next-updating-rendering-time.html.ini deleted file mode 100644 index dcc8d0052c0..00000000000 --- a/tests/wpt/meta/html/webappapis/scripting/event-loops/new-scroll-event-dispatched-at-next-updating-rendering-time.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[new-scroll-event-dispatched-at-next-updating-rendering-time.html] - expected: TIMEOUT - [new-scroll-event-dispatched-at-next-updating-rendering-time] - expected: TIMEOUT diff --git a/tests/wpt/tests/dom/events/resources/large-dimension-document.sub.html b/tests/wpt/tests/dom/events/resources/large-dimension-document.sub.html new file mode 100644 index 00000000000..5d29f7be0bb --- /dev/null +++ b/tests/wpt/tests/dom/events/resources/large-dimension-document.sub.html @@ -0,0 +1,5 @@ + + + +
+ diff --git a/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-element.html b/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-element.html new file mode 100644 index 00000000000..3484a2ec92b --- /dev/null +++ b/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-element.html @@ -0,0 +1,100 @@ + + +Scroll event should behave correctly for Element.offsetTop and Element.offsetLeft + + + + + + +
+
+
+ diff --git a/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-iframe.html b/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-iframe.html new file mode 100644 index 00000000000..2ff3de79d33 --- /dev/null +++ b/tests/wpt/tests/dom/events/scrolling/scroll-event-fired-to-iframe.html @@ -0,0 +1,116 @@ + + +Scroll event should behave correctly for Element.ScrollX and Element.ScrollLeft + + + + + + +
+
+
+