Skip to content

Commit 71c61fa

Browse files
authored
Merge pull request #1498 from jaytaph/gos-282-no-content
GOS-282: a 204/205 navigation keeps the current document
2 parents d7d2def + c743835 commit 71c61fa

3 files changed

Lines changed: 185 additions & 21 deletions

File tree

‎crates/gosub_engine/src/engine/engine.rs‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3883,6 +3883,125 @@ mod tests {
38833883
.expect("the page's own stylesheet was refused");
38843884
}
38853885

3886+
/// A navigation answered 204 or 205 ends without a download offer, an error or a new
3887+
/// document: the tab gets Cancelled and keeps its page. A traversal answered 204 leaves
3888+
/// the history cursor on the page that stays.
3889+
#[tokio::test]
3890+
async fn navigation_to_no_content_keeps_the_page() {
3891+
use crate::events::NavigationEvent;
3892+
use std::sync::atomic::{AtomicUsize, Ordering};
3893+
use tokio::io::{AsyncReadExt, AsyncWriteExt};
3894+
3895+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
3896+
let port = listener.local_addr().unwrap().port();
3897+
// `/flip` is a page the first time and 204 after that.
3898+
let flips = Arc::new(AtomicUsize::new(0));
3899+
tokio::spawn(async move {
3900+
while let Ok((mut stream, _)) = listener.accept().await {
3901+
let flips = flips.clone();
3902+
tokio::spawn(async move {
3903+
let mut buf = vec![0u8; 4096];
3904+
let n = stream.read(&mut buf).await.unwrap_or(0);
3905+
let flip = buf[..n].starts_with(b"GET /flip ");
3906+
let gone = flip && flips.fetch_add(1, Ordering::SeqCst) > 0;
3907+
let head = if buf[..n].starts_with(b"GET /204 ") || gone {
3908+
"HTTP/1.1 204 No Content\r\nConnection: close\r\n\r\n".to_string()
3909+
} else if buf[..n].starts_with(b"GET /205 ") {
3910+
"HTTP/1.1 205 Reset Content\r\nContent-Length: 0\r\nConnection: close\r\n\r\n".to_string()
3911+
} else {
3912+
let page = "<html><head><title>kept</title></head><body>page</body></html>";
3913+
format!(
3914+
"HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\
3915+
Content-Length: {}\r\nConnection: close\r\n\r\n{page}",
3916+
page.len()
3917+
)
3918+
};
3919+
let _ = stream.write_all(head.as_bytes()).await;
3920+
});
3921+
}
3922+
});
3923+
3924+
let mut engine = engine_with_max_zones(1);
3925+
let mut events = engine.subscribe_events();
3926+
let _join = tokio::spawn(engine.start().expect("start"));
3927+
let mut zone = engine.zone_builder().services(services()).create().expect("zone");
3928+
let tab = zone.tab_builder().create().await.expect("tab");
3929+
3930+
// Every event up to the end of the next navigation; panics on an offer or a failure.
3931+
async fn next_end(events: &mut broadcast::Receiver<EngineEvent>) -> NavigationEvent {
3932+
tokio::time::timeout(Duration::from_secs(10), async {
3933+
loop {
3934+
match events.recv().await {
3935+
Ok(EngineEvent::DownloadRequested { url, .. }) => panic!("download offered for {url}"),
3936+
Ok(EngineEvent::Navigation {
3937+
event: event @ (NavigationEvent::Finished { .. } | NavigationEvent::Cancelled { .. }),
3938+
..
3939+
}) => return event,
3940+
Ok(EngineEvent::Navigation {
3941+
event: NavigationEvent::Failed { error, .. },
3942+
..
3943+
}) => panic!("navigation failed: {error}"),
3944+
Ok(_) => continue,
3945+
Err(e) => panic!("event stream closed: {e}"),
3946+
}
3947+
}
3948+
})
3949+
.await
3950+
.expect("timed out waiting for the navigation to end")
3951+
}
3952+
3953+
tab.navigate(format!("http://127.0.0.1:{port}/page"))
3954+
.await
3955+
.expect("navigate");
3956+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Finished { .. }));
3957+
3958+
for status in ["204", "205"] {
3959+
tab.navigate(format!("http://127.0.0.1:{port}/{status}"))
3960+
.await
3961+
.expect("navigate");
3962+
match next_end(&mut events).await {
3963+
NavigationEvent::Cancelled { url, .. } => assert_eq!(url.path(), format!("/{status}")),
3964+
other => panic!("{status}: expected Cancelled, got {other:?}"),
3965+
}
3966+
assert_eq!(tab.url().map(|u| u.path().to_string()).as_deref(), Some("/page"));
3967+
assert_eq!(tab.title(), "kept");
3968+
}
3969+
3970+
// Nothing was left pending behind the cancel: the next navigation is a plain one.
3971+
tab.navigate(format!("http://127.0.0.1:{port}/page"))
3972+
.await
3973+
.expect("navigate");
3974+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Finished { .. }));
3975+
assert!(tab.pending_downloads().is_empty());
3976+
3977+
// Forward to an entry that now answers 204: the cursor stays on the page shown.
3978+
tab.navigate(format!("http://127.0.0.1:{port}/flip"))
3979+
.await
3980+
.expect("navigate");
3981+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Finished { .. }));
3982+
tab.go_back().await.expect("go back");
3983+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Finished { .. }));
3984+
tab.go_forward().await.expect("go forward");
3985+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Cancelled { .. }));
3986+
assert_eq!(tab.url().map(|u| u.path().to_string()).as_deref(), Some("/page"));
3987+
assert!(tab.can_go_forward(), "the cursor stayed on /flip");
3988+
3989+
// After a fragment navigation the page shown is on the fragment's entry.
3990+
tab.navigate(format!("http://127.0.0.1:{port}/page#s"))
3991+
.await
3992+
.expect("navigate");
3993+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Finished { .. }));
3994+
tab.navigate(format!("http://127.0.0.1:{port}/204"))
3995+
.await
3996+
.expect("navigate");
3997+
assert!(matches!(next_end(&mut events).await, NavigationEvent::Cancelled { .. }));
3998+
assert_eq!(
3999+
tab.url().and_then(|u| u.fragment().map(str::to_string)).as_deref(),
4000+
Some("s")
4001+
);
4002+
assert!(tab.can_go_back(), "the cursor went back to /page's entry");
4003+
}
4004+
38864005
/// A reload ignoring the cache asks for a fresh copy all the way to the server
38874006
/// (`Cache-Control` and `Pragma: no-cache`, no validators); a plain reload does not.
38884007
#[tokio::test]

‎crates/gosub_engine/src/engine/tab/worker.rs‎

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ pub enum NavigationResult<C: RenderConfiguration> {
226226
meta: FetchResultMeta,
227227
spool: crate::net::Spool,
228228
},
229+
/// The response was 204 or 205: the navigation ends and the page stays.
230+
NoContent { nav_id: NavigationId, url: Url },
229231
}
230232

231233
// Current active navigation
@@ -375,9 +377,10 @@ pub struct TabWorker<C: RenderConfiguration> {
375377
/// the renders that follow do not fetch it again. Cleared when a navigation
376378
/// commits a new document.
377379
remote_favicon: Option<Url>,
378-
/// The history entry the shown document was committed to. A traversal
379-
/// moves the cursor before its load commits, and a title the renderer
380-
/// reports meanwhile is still this document's, not the cursor's.
380+
/// The history entry the shown page is on: the one its document was
381+
/// committed to, or a fragment navigation's since. A traversal moves the
382+
/// cursor before its load commits, and a title the renderer reports
383+
/// meanwhile is still this document's, not the cursor's.
381384
document_entry: Option<HistoryEntryId>,
382385
}
383386

@@ -781,6 +784,45 @@ impl<C: RenderConfiguration> TabWorker<C> {
781784
self.context.has_remote_renderer()
782785
}
783786

787+
/// End a navigation that leaves the current document showing. Not an error: the
788+
/// Cancelled event stops the shell's spinner and puts its address bar back.
789+
fn end_navigation_in_place(&mut self, nav_id: NavigationId, url: Url, reason: &str) {
790+
self.sink.security.abandon(nav_id);
791+
// A superseded navigation's result: the tab's state is the newer one's.
792+
if self.active_nav.as_ref().is_some_and(|a| a.nav_id == nav_id) {
793+
self.active_nav = None;
794+
self.load = None;
795+
self.is_loading = false;
796+
self.is_error = false;
797+
self.state = TabState::Idle;
798+
self.pending_url = None;
799+
self.return_to_document_entry();
800+
}
801+
self.send_event(EngineEvent::Navigation {
802+
tab_id: self.tab_id,
803+
event: NavigationEvent::Cancelled {
804+
nav_id,
805+
url,
806+
reason: crate::engine::events::CancelReason::Custom(reason.into()),
807+
},
808+
});
809+
}
810+
811+
/// Undo what the start of a navigation did to the page that stays: a traversal moved the
812+
/// history cursor, and every navigation scrolled to the top. The offset is the one saved
813+
/// on the document's entry when the navigation began.
814+
fn return_to_document_entry(&mut self) {
815+
let Some(entry) = self.document_entry else {
816+
return;
817+
};
818+
if self.history.current() != Some(entry) && self.history.go_to(entry).is_some() {
819+
self.emit_history_changed();
820+
}
821+
if let Some((x, y)) = self.history.entry(entry).map(|e| e.scroll) {
822+
self.apply_scroll(x, y);
823+
}
824+
}
825+
784826
fn on_nav_result(&mut self, res: NavigationResult<C>) {
785827
match res {
786828
NavigationResult::Ok {
@@ -925,29 +967,16 @@ impl<C: RenderConfiguration> TabWorker<C> {
925967
self.remote_capture = false;
926968
self.remote_focus = None;
927969
}
970+
NavigationResult::NoContent { nav_id, url } => {
971+
self.end_navigation_in_place(nav_id, url, "no content");
972+
}
928973
NavigationResult::Download { nav_id, meta, spool } => {
929-
self.sink.security.abandon(nav_id);
930974
// Not an error and not a page change: the tab stays on its current document
931-
// and the shell gets a download offer. The Cancelled event stops spinners.
975+
// and the shell gets a download offer.
932976
// The navigation ends here, without cancelling it: its token is the fetch's
933977
// parent, so the next navigation or a CancelNavigation would stop the body
934978
// under an accepted download. The spool owns the fetch from now on.
935-
if self.active_nav.as_ref().is_some_and(|a| a.nav_id == nav_id) {
936-
self.active_nav = None;
937-
self.load = None;
938-
}
939-
self.is_loading = false;
940-
self.is_error = false;
941-
self.state = TabState::Idle;
942-
self.pending_url = None;
943-
self.send_event(EngineEvent::Navigation {
944-
tab_id: self.tab_id,
945-
event: NavigationEvent::Cancelled {
946-
nav_id,
947-
url: meta.final_url.clone(),
948-
reason: crate::engine::events::CancelReason::Custom("download".into()),
949-
},
950-
});
979+
self.end_navigation_in_place(nav_id, meta.final_url.clone(), "download");
951980
let info = crate::net::types::ResponseInfo::from(&meta);
952981
self.next_offer += 1;
953982
let pending = PendingDownload {
@@ -2070,6 +2099,7 @@ impl<C: RenderConfiguration> TabWorker<C> {
20702099
}
20712100
HistoryIntent::Reload | HistoryIntent::Replace | HistoryIntent::Traverse(_) => {}
20722101
}
2102+
self.document_entry = self.history.current();
20732103
self.current_url = Some(url.clone());
20742104

20752105
// Layout already exists, so the target can be scrolled to right away. A traversal to
@@ -2451,6 +2481,12 @@ impl<C: RenderConfiguration> TabWorker<C> {
24512481
spool,
24522482
});
24532483
}
2484+
Ok(RoutedOutcome::NoContent) => {
2485+
let _ = tx_done.send(NavigationResult::NoContent {
2486+
nav_id,
2487+
url: document_final_url,
2488+
});
2489+
}
24542490
Ok(RoutedOutcome::ViewerRendered(_doc)) => {
24552491
log::warn!("Tab[{:?}] viewer rendering not supported yet", tab_id);
24562492
let _ = tx_done.send(NavigationResult::Err {

‎crates/gosub_engine/src/net/router.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ pub enum RoutedOutcome<C: RenderConfiguration> {
5555
meta: Box<crate::net::types::FetchResultMeta>,
5656
spool: Spool,
5757
},
58+
59+
/// A top-level navigation answered 204 or 205: the navigation ends and the current
60+
/// document stays (HTML, "process a navigate response").
61+
NoContent,
5862
}
5963

6064
/// The disk held by download spools across every tab of an engine.
@@ -479,6 +483,11 @@ pub async fn route_response_for<C: RenderConfiguration>(
479483
}
480484
};
481485

486+
if dest == RequestDestination::Document && matches!(meta.status, 204 | 205) {
487+
handle.cancel.cancel();
488+
return Ok(RoutedOutcome::NoContent);
489+
}
490+
482491
let outcome = decide_handling(&meta, dest, peek_buf.clone(), policy);
483492

484493
match (dest, outcome.decision, body_content) {

0 commit comments

Comments
 (0)