From b1b7ccfbd22078d640e261fb6a1878bec02102d4 Mon Sep 17 00:00:00 2001
From: Kamil Jarosz <kjarosh256@gmail.com>
Date: Fri, 10 May 2024 19:55:29 +0200
Subject: [PATCH] avm1: Highlight buttons using their hit bounds

Buttons are always highlighted using their hit bounds.
I guess it does have some sense to it, because their bounds
usually change on hover (children are swapped out),
which would cause the automatic tab order to change during tabbing.
That could potentially create a loop in the tab ordering
(soft locking the tab).
---
 core/src/display_object/avm1_button.rs | 21 ++++++++++++++++-----
 swf/src/types/rectangle.rs             |  2 +-
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/core/src/display_object/avm1_button.rs b/core/src/display_object/avm1_button.rs
index 712dfef4a..65431e434 100644
--- a/core/src/display_object/avm1_button.rs
+++ b/core/src/display_object/avm1_button.rs
@@ -52,6 +52,8 @@ pub struct Avm1ButtonData<'gc> {
 struct Avm1ButtonDataMut<'gc> {
     base: InteractiveObjectBase<'gc>,
     hit_area: BTreeMap<Depth, DisplayObject<'gc>>,
+    #[collect(require_static)]
+    hit_bounds: Rectangle<Twips>,
     container: ChildContainer<'gc>,
 }
 
@@ -73,6 +75,7 @@ impl<'gc> Avm1Button<'gc> {
                     base: Default::default(),
                     container: ChildContainer::new(source_movie.movie.clone()),
                     hit_area: BTreeMap::new(),
+                    hit_bounds: Default::default(),
                 }),
                 static_data: Gc::new(
                     mc,
@@ -126,11 +129,7 @@ impl<'gc> Avm1Button<'gc> {
     ///
     /// This function instantiates children and thus must not be called whilst
     /// the caller is holding a write lock on the button data.
-    pub fn set_state(
-        mut self,
-        context: &mut crate::context::UpdateContext<'_, 'gc>,
-        state: ButtonState,
-    ) {
+    pub fn set_state(mut self, context: &mut UpdateContext<'_, 'gc>, state: ButtonState) {
         let mut removed_depths: fnv::FnvHashSet<_> =
             self.iter_render_list().map(|o| o.depth()).collect();
 
@@ -335,10 +334,13 @@ impl<'gc> TDisplayObject<'gc> for Avm1Button<'gc> {
             }
 
             let write = unlock!(Gc::write(context.gc(), self.0), Avm1ButtonData, cell);
+            let mut hit_bounds = Rectangle::INVALID;
             for (child, depth) in new_children {
                 child.post_instantiation(context, None, Instantiator::Movie, false);
                 write.borrow_mut().hit_area.insert(depth, child);
+                hit_bounds = hit_bounds.union(&child.local_bounds());
             }
+            write.borrow_mut().hit_bounds = hit_bounds;
         }
     }
 
@@ -600,6 +602,15 @@ impl<'gc> TInteractiveObject<'gc> for Avm1Button<'gc> {
     fn tab_enabled_avm1(&self, context: &mut UpdateContext<'_, 'gc>) -> bool {
         self.get_avm1_boolean_property(context, "tabEnabled", |_| true)
     }
+
+    fn highlight_bounds(self) -> Rectangle<Twips> {
+        // Buttons are always highlighted using their hit bounds.
+        // I guess it does have some sense to it, because their bounds
+        // usually change on hover (children are swapped out),
+        // which would cause the automatic tab order to change during tabbing.
+        // That could potentially create a loop in the tab ordering (soft locking the tab).
+        self.local_to_global_matrix() * self.0.cell.borrow().hit_bounds.clone()
+    }
 }
 
 impl<'gc> Avm1ButtonData<'gc> {
diff --git a/swf/src/types/rectangle.rs b/swf/src/types/rectangle.rs
index 2479290ff..f0b353939 100644
--- a/swf/src/types/rectangle.rs
+++ b/swf/src/types/rectangle.rs
@@ -58,7 +58,7 @@ impl<T: PointCoordinate + Ord> Rectangle<T> {
 }
 
 impl<T: Coordinate> Rectangle<T> {
-    const INVALID: Self = Self {
+    pub const INVALID: Self = Self {
         x_min: T::INVALID,
         x_max: T::INVALID,
         y_min: T::INVALID,