Skip to content

script: Implement fully exit fullscreen steps partially - #47123

Merged
jdm merged 2 commits into
servo:mainfrom
shubhamg13:full_exit_fullscreen
Aug 11, 2026
Merged

jdm merged 2 commits into
servo:mainfrom
shubhamg13:full_exit_fullscreen

Conversation

@shubhamg13

@shubhamg13 shubhamg13 commented Aug 10, 2026 •

Copy link
Copy Markdown
Member

Implement fully exit fullscreen steps

Testing: tests/webview.rs: test_fullscreen
Fixes: #45157

Signed-off-by: Shubham Gupta <shubhamg13.work@gmail.com>
@shubhamg13
shubhamg13 requested a review from gterzian as a code owner August 10, 2026 08:43
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 10, 2026
@shubhamg13 shubhamg13 changed the title Implement fully exit fullscreen steps script: Implement fully exit fullscreen steps Aug 10, 2026
@shubhamg13 shubhamg13 changed the title script: Implement fully exit fullscreen steps script: Implement fully exit fullscreen steps partially Aug 10, 2026

@Narfinger Narfinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems ok but I am not that familiar with the material.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 10, 2026
@jdm

jdm commented Aug 11, 2026

Copy link
Copy Markdown
Member

Testing: Tested using the data URL
data:text/html,<script>addEventListener("keypress",()=>{document.documentElement.requestFullscreen()})</script>link

Please remember that the Testing field in the template should explicitly address any automated tests that do or do not exist for the changes.

@jdm

jdm commented Aug 11, 2026

Copy link
Copy Markdown
Member
diff --git a/components/servo/tests/common/mod.rs b/components/servo/tests/common/mod.rs
index dbce0574f59..4305cb50696 100644
--- a/components/servo/tests/common/mod.rs
+++ b/components/servo/tests/common/mod.rs
@@ -97,6 +97,7 @@ pub(crate) struct WebViewDelegateImpl {
     pub(crate) number_of_controls_hidden: Cell<usize>,
     pub(crate) last_accesskit_tree_updates: RefCell<Vec<accesskit::TreeUpdate>>,
     pub(crate) console_messages: RefCell<Vec<(ConsoleLogLevel, String)>>,
+    pub(crate) fullscreen: Cell<bool>,
 }
 
 #[allow(dead_code)] // Used by some tests and not others
@@ -165,6 +166,10 @@ impl WebViewDelegate for WebViewDelegateImpl {
     fn show_console_message(&self, _webview: WebView, level: ConsoleLogLevel, message: String) {
         self.console_messages.borrow_mut().push((level, message));
     }
+
+    fn notify_fullscreen_state_changed(&self, _webview: WebView, is_fullscreen: bool) {
+        self.fullscreen.set(is_fullscreen);
+    }
 }
 
 // Used by some unit tests only. Since they compile into different binaries,
diff --git a/components/servo/tests/webview.rs b/components/servo/tests/webview.rs
index 6d8f74ebf60..a8b3d9f1ebe 100644
--- a/components/servo/tests/webview.rs
+++ b/components/servo/tests/webview.rs
@@ -1134,3 +1134,39 @@ fn test_preferences_change() {
         )
     );
 }
+
+#[test]
+fn test_fullscreen() {
+    let servo_test = ServoTest::new();
+    let delegate = Rc::new(WebViewDelegateImpl::default());
+
+    let test_page = Url::parse(
+        "data:text/html,\
+        <button onclick='this.requestFullscreen()'>click</button>\
+        <a href=about:blank>click</a>",
+    )
+    .expect("Data URL failed to build");
+
+    let webview = WebViewBuilder::new(servo_test.servo(), servo_test.rendering_context.clone())
+        .delegate(delegate.clone())
+        .url(test_page.clone())
+        .build();
+    show_webview_and_wait_for_rendering_to_be_ready(&servo_test, &webview, &delegate);
+
+    click_at_point(&webview, Point2D::new(10., 10.));
+
+    let captured = delegate.clone();
+    servo_test.spin(move || !captured.fullscreen.get());
+
+    assert!(
+        evaluate_javascript(
+            &servo_test,
+            webview.clone(),
+            "document.querySelector('a').click();"
+        )
+        .is_ok()
+    );
+
+    let captured = delegate.clone();
+    servo_test.spin(move || captured.fullscreen.get());
+}

Here's a test that should pass with your changes :)

@jdm jdm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good assuming the test passes!

@jdm jdm added the S-needs-tests New tests have been requested by a reviewer. label Aug 11, 2026
Signed-off-by: Shubham Gupta <shubhamg13.work@gmail.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
@jdm
jdm added this pull request to the merge queue Aug 11, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 11, 2026
Merged via the queue into servo:main with commit 4d6c61c Aug 11, 2026
34 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 11, 2026
@shubhamg13
shubhamg13 deleted the full_exit_fullscreen branch August 11, 2026 08:30
RichardTjokroutomo pushed a commit to RichardTjokroutomo/servo that referenced this pull request Oct 6, 2026
Implement `fully exit fullscreen` steps

Testing: `tests/webview.rs: test_fullscreen`
Fixes: servo#45157

---------

Signed-off-by: Shubham Gupta <shubhamg13.work@gmail.com>
Co-authored-by: Josh Matthews <josh@joshmatthews.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-needs-tests New tests have been requested by a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

when fullscreen by web API, navigating should exit fullscreen

4 participants