hanna/fabrica

fix(web): comment form no longer nests forms; issue label + control fixes

a4ddbe6 · hanna committed on 2026-07-25

Fix the "duplicate field _csrf" error when commenting: the Close/Reopen state
form was nested inside the comment form (invalid HTML). The comment textarea is
now its own form and the Comment button references it via form=, with the state
form a sibling. Style number inputs globally (the "Blocked by" #123 box),
refine dependency rows, opt sidebar/action buttons out of the stacked-form
margin (inline-btn), auto-apply labels on checkbox change, and add a multi-label
dropdown to the new-issue form (labels applied on create).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Signed by hannaSSH key fingerprint: SHA256:4g9cWhkAAw8gwqhJUcVbSnGEvdGwklW+1Aa/fMUu59k

assets/base.css +49 −5

assets/fabrica.js +6 −6

crates/web/src/issues.rs +74 −32

crates/web/src/pulls.rs +7 −8

crates/web/src/tests.rs +73 −2

   color: var(--fb-fg);
   white-space: nowrap;
+}
+/* Multi-label dropdown on the new-issue form. */
+.label-dropdown {
+  display: inline-block;
+  position: relative;
+}
+.label-dropdown > summary {
+  list-style: none;
+  width: auto;
+  cursor: pointer;
+}
+.label-dropdown > summary::-webkit-details-marker {
+  display: none;
+}
+.label-menu {
+  left: 0;
+  right: auto;
+  display: flex;
+  flex-direction: column;
+  gap: 0.35rem;
+  max-height: 16rem;
+  overflow-y: auto;
+}
+.label-menu .checkbox {
+  margin: 0;
+}
 .menu-item:hover {
   background: var(--fb-bg-inset);
   text-decoration: none;
 /* Issue/PR dependency lists in the sidebar. */
 .dep-list {
   list-style: none;
+  padding: 0;
   display: flex;
   flex-direction: column;
-  gap: 0.35rem;
+  gap: 0.25rem;
-  margin-bottom: 0.5rem;
+  margin: 0 0 0.75rem;
+}
 .dep-row {
   display: flex;
   align-items: center;
-  gap: 0.4rem;
+  gap: 0.45rem;
   font-size: 0.9em;
+  padding: 0.3rem 0.5rem;
+  border-radius: var(--fb-radius);
+}
+.dep-row:hover {
+  background: var(--fb-bg-inset);
+}
+.dep-row > .icon {
+  flex: none;
+}
 .dep-row > a {
   flex: 1;
   text-overflow: ellipsis;
   white-space: nowrap;
+}
+.dep-row > a:hover {
+  text-decoration: none;
+  color: var(--fb-accent);
+}
 .dep-row .icon-btn {
-  width: 1.5rem;
+  width: 1.4rem;
-  height: 1.5rem;
+  height: 1.4rem;
   flex: none;
+  color: var(--fb-fg-muted);
+}
+.dep-row .icon-btn:hover {
+  color: var(--fb-danger);
+}
 .dep-add {
   display: flex;
   gap: 0.4rem;
+  margin-top: 0.25rem;
+}
 .dep-add input {
   flex: 1;
 input[type="email"],
 input[type="search"],
 input[type="url"],
+input[type="number"],
 textarea,
 select {
   width: 100%;

     });
   });
-  // ---- Auto-submitting selects ----
+  // ---- Auto-submitting controls ----
-  // A <select data-autosubmit> submits its form on change; without JS a noscript
+  // Any [data-autosubmit] control (select or checkbox) submits its form on
-  // Apply button does the same.
+  // change; without JS a noscript button does the same.
-  document.querySelectorAll("select[data-autosubmit]").forEach(function (sel) {
+  document.querySelectorAll("[data-autosubmit]").forEach(function (el) {
-    sel.addEventListener("change", function () {
+    el.addEventListener("change", function () {
-      if (sel.form) sel.form.submit();
+      if (el.form) el.form.submit();
     });
   });

         return Ok(Redirect::to("/login").into_response());
+    }
     let branches = crate::repo::branch_names(state, &ctx.repo.id).await;
+    let repo_labels = state.store.list_labels(&ctx.repo.id).await?;
     let (jar, chrome) = build_chrome(state, jar, viewer, uri.path().to_string());
     let body = html! {
         (repo_header(state, &ctx, kind.tab, &ctx.repo.default_branch, &branches))
                 input type="text" id="title" name="title" required autofocus;
                 label for="body" { "Description" }
                 textarea id="body" name="body" rows="8" placeholder="Markdown supported" {}
+                @if !repo_labels.is_empty() {
+                    label { "Labels" }
+                    (label_dropdown(&repo_labels))
+                }
                 button class="btn btn-primary" type="submit" { "Create " (kind.noun) }
+            }
+        }
                     @for (c, cauthor) in &comment_rows {
                         (comment_card(cauthor, &c.body, c.created_at))
+                    }
-                    @if can_write {
+                    @if can_write || is_author {
+                        @let comment_form = format!("comment-{}", issue.id);
                         div class="card comment-form" {
-                            form method="post" action=(format!("/issue/{}/comment", issue.id)) {
+                            // The comment form holds only the textarea; the Comment
+                            // button and the (separate) state form are siblings in
+                            // the actions row — never nested forms.
+                            form id=(comment_form) method="post" action=(format!("/issue/{}/comment", issue.id)) {
                                 input type="hidden" name="_csrf" value=(csrf);
                                 textarea name="body" rows="4" placeholder="Leave a comment (markdown)" required {}
-                                div class="comment-actions" {
+                            }
-                                    @if can_write || is_author {
+                            div class="comment-actions" {
-                                        (state_button(&issue, &csrf, kind))
+                                (state_button(&issue, &csrf, kind))
-                                    }
+                                button class="btn btn-primary inline-btn" type="submit" form=(comment_form) { "Comment" }
-                                    button class="btn btn-primary" type="submit" { "Comment" }
-                                }
+                            }
+                        }
+                    }
         form method="post" action=(format!("/issue/{}/state", issue.id)) class="inline-form" {
             input type="hidden" name="_csrf" value=(csrf);
             input type="hidden" name="state" value=(target);
-            button class="btn" type="submit" { (label) }
+            button class="btn inline-btn" type="submit" { (label) }
+        }
+    }
+}
                     option value=(id) selected[issue.assignee_id.as_deref() == Some(id.as_str())] { (name) }
+                }
+            }
-            button class="btn" type="submit" { "Set" }
+            button class="btn inline-btn" type="submit" { "Set" }
+        }
+    }
+}
 ) -> Markup {
     let is_on = |id: &str| applied.iter().any(|l| l.id == id);
     html! {
+        // Each checkbox auto-submits the whole set (the handler replaces the
+        // applied labels); a noscript Apply button keeps it working without JS.
         form method="post" action=(format!("/issue/{}/labels", issue.id)) class="side-form label-picker" {
             input type="hidden" name="_csrf" value=(csrf);
             @for l in repo_labels {
                 label class="checkbox" {
-                    input type="checkbox" name="label" value=(l.id) checked[is_on(&l.id)];
+                    input type="checkbox" name="label" value=(l.id) checked[is_on(&l.id)]
+                          data-autosubmit;
                     " " span class="label-swatch" style=(format!("--label-color: {}", css_color(&l.color))) {}
                     " " (l.name)
+                }
+            }
-            button class="btn" type="submit" { "Apply labels" }
+            noscript { button class="btn inline-btn" type="submit" { "Apply labels" } }
+        }
+    }
+}
+/// A `<details>` dropdown of label checkboxes, for selecting several labels when
+/// opening an issue. The checkboxes submit with the surrounding form.
+fn label_dropdown(repo_labels: &[Label]) -> Markup {
+    html! {
+        details class="menu label-dropdown" data-menu {
+            summary class="btn" { "Select labels" (icon(Icon::ChevronDown)) }
+            div class="menu-popover label-menu" {
+                @for l in repo_labels {
+                    label class="checkbox" {
+                        input type="checkbox" name="label" value=(l.id);
+                        " " span class="label-swatch" style=(format!("--label-color: {}", css_color(&l.color))) {}
+                        " " (l.name)
+                    }
+                }
+            }
+        }
+    }
+}
                     form method="post" action=(format!("/issue/{}/deps/remove", issue.id)) class="dep-remove" {
                         input type="hidden" name="_csrf" value=(csrf);
                         input type="hidden" name="depends_on" value=(dep.id);
-                        button class="icon-btn" type="submit" aria-label="Remove dependency" { (icon(Icon::X)) }
+                        button class="icon-btn inline-btn" type="submit" aria-label="Remove dependency" { (icon(Icon::X)) }
+                    }
+                }
+            }
                 form method="post" action=(format!("/issue/{}/deps/add", issue.id)) class="side-form dep-add" {
                     input type="hidden" name="_csrf" value=(csrf);
                     input type="number" name="number" min="1" placeholder="#123" aria-label="Depends on number" required;
-                    button class="btn" type="submit" { "Add" }
+                    button class="btn inline-btn" type="submit" { "Add" }
+                }
+            }
             @if !dependents.is_empty() {
     format!("/{owner}/{}/-/{seg}/{}", repo.path, issue.number)
+}
-/// New-issue form fields + a query flag for `is_pull`.
-#[derive(Debug, Deserialize)]
-pub struct NewIssueForm {
-    /// Title.
-    #[serde(default)]
-    title: String,
-    /// Markdown body.
-    #[serde(default)]
-    body: String,
-    /// CSRF token.
-    #[serde(rename = "_csrf")]
-    csrf: String,
-}
 /// `?is_pull=` query for the create endpoint.
 #[derive(Debug, Default, Deserialize)]
 pub struct CreateQuery {
+}
 /// `POST /issue-new/{repo_id}` — create an issue (PRs use their own flow).
+///
+/// The body is parsed manually because it carries repeated `label` fields, which
+/// `serde_urlencoded` cannot represent.
 pub async fn create(
     State(state): State<AppState>,
     MaybeUser(viewer): MaybeUser,
     headers: HeaderMap,
     Path(repo_id): Path<String>,
     Query(q): Query<CreateQuery>,
-    Form(form): Form<NewIssueForm>,
+    body: axum::body::Bytes,
 ) -> Response {
-    if verify_csrf(&jar, &headers, Some(&form.csrf)).is_err() {
+    let (mut title, mut desc, mut csrf) = (String::new(), String::new(), String::new());
+    let mut chosen: Vec<String> = Vec::new();
+    for (k, v) in url::form_urlencoded::parse(&body) {
+        match k.as_ref() {
+            "title" => title = v.into_owned(),
+            "body" => desc = v.into_owned(),
+            "_csrf" => csrf = v.into_owned(),
+            "label" => chosen.push(v.into_owned()),
+            _ => {}
+        }
+    }
+    if verify_csrf(&jar, &headers, Some(&csrf)).is_err() {
         return AppError::Forbidden.into_response();
+    }
     let result = async {
         if access_for(&state, Some(&user), &repo).await? < auth::Access::Read {
             return Err(AppError::NotFound);
+        }
-        let title = form.title.trim();
+        let title = title.trim();
         if title.is_empty() {
             return Err(AppError::BadRequest("A title is required.".to_string()));
+        }
         let issue = state
             .store
-            .create_issue(&repo.id, &user.id, title, form.body.trim(), false)
+            .create_issue(&repo.id, &user.id, title, desc.trim(), false)
             .await?;
+        // Apply any selected labels that belong to this repo.
+        if !chosen.is_empty() {
+            let valid: std::collections::HashSet<String> = state
+                .store
+                .list_labels(&repo.id)
+                .await?
+                .into_iter()
+                .map(|l| l.id)
+                .collect();
+            for id in chosen.iter().filter(|id| valid.contains(*id)) {
+                state.store.add_issue_label(&issue.id, id).await?;
+            }
+        }
         Ok::<String, AppError>(issue_url(&state, &repo, &issue).await)
+    }
     .await;

@@ -154,17 +154,16 @@ pub(crate) async fn view(
154	(comment_card(cauthor, &c.body, c.created_at))	154	(comment_card(cauthor, &c.body, c.created_at))
155	}	155	}
156	(merge_panel(&issue, &pr, mergeable, blocked, &csrf))	156	(merge_panel(&issue, &pr, mergeable, blocked, &csrf))
157	@if can_write {	157	@if can_write \|\| is_author {
		158	@let comment_form = format!("comment-{}", issue.id);
158	div class="card comment-form" {	159	div class="card comment-form" {
159	form method="post" action=(format!("/issue/{}/comment", issue.id)) {	160	form id=(comment_form) method="post" action=(format!("/issue/{}/comment", issue.id)) {
160	input type="hidden" name="_csrf" value=(csrf);	161	input type="hidden" name="_csrf" value=(csrf);
161	textarea name="body" rows="4" placeholder="Leave a comment (markdown)" required {}	162	textarea name="body" rows="4" placeholder="Leave a comment (markdown)" required {}
162	div class="comment-actions" {	163	}
163	@if can_write \|\| is_author {	164	div class="comment-actions" {
164	(state_button(&issue, &csrf, Kind::pulls()))	165	(state_button(&issue, &csrf, Kind::pulls()))
165	}	166	button class="btn btn-primary inline-btn" type="submit" form=(comment_form) { "Comment" }
166	button class="btn btn-primary" type="submit" { "Comment" }
167	}
168	}	167	}
169	}	168	}
170	}	169	}

     // View it.
     let req = Request::builder()
         .uri("/ada/proj/-/issues/1")
-        .header(header::COOKIE, cookie)
+        .header(header::COOKIE, cookie.clone())
         .body(Body::empty())
         .unwrap();
-    let res = app.oneshot(req).await.unwrap();
+    let res = app.clone().oneshot(req).await.unwrap();
     assert_eq!(res.status(), StatusCode::OK);
     let html = body_string(res).await;
     assert!(html.contains("First bug"), "issue title shown");
     assert!(html.contains("#1"), "issue number shown");
+    // Commenting works (regression: nested state form used to duplicate _csrf).
+    let issue = state
+        .store
+        .issue_by_number(&repo.id, 1, false)
+        .await
+        .unwrap()
+        .unwrap();
+    let req = Request::builder()
+        .method("POST")
+        .uri(format!("/issue/{}/comment", issue.id))
+        .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded")
+        .header(header::COOKIE, cookie)
+        .body(Body::from(format!("body=a+comment&_csrf={token}")))
+        .unwrap();
+    let res = app.clone().oneshot(req).await.unwrap();
+    assert_eq!(res.status(), StatusCode::SEE_OTHER, "comment accepted");
+    assert_eq!(state.store.list_comments(&issue.id).await.unwrap().len(), 1);
+}
+#[tokio::test]
+async fn new_issue_can_apply_labels() {
+    let (state, app) = app().await;
+    let ada = state.store.user_by_username("ada").await.unwrap().unwrap();
+    let repo = state
+        .store
+        .create_repo(NewRepo {
+            owner_id: ada.id.clone(),
+            group_id: None,
+            name: "proj".to_string(),
+            path: "proj".to_string(),
+            description: None,
+            visibility: model::Visibility::Public,
+            default_branch: "main".to_string(),
+        })
+        .await
+        .unwrap();
+    let bug = state
+        .store
+        .create_label(&repo.id, "bug", "#ff0000", None)
+        .await
+        .unwrap();
+    let cookie = login(&app).await;
+    let token = cookie
+        .split("fabrica_csrf=")
+        .nth(1)
+        .and_then(|s| s.split(';').next())
+        .unwrap()
+        .to_string();
+    let req = Request::builder()
+        .method("POST")
+        .uri(format!("/issue-new/{}", repo.id))
+        .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded")
+        .header(header::COOKIE, cookie)
+        .body(Body::from(format!(
+            "title=Tagged&body=&label={}&_csrf={token}",
+            bug.id
+        )))
+        .unwrap();
+    let res = app.oneshot(req).await.unwrap();
+    assert_eq!(res.status(), StatusCode::SEE_OTHER);
+    let issue = state
+        .store
+        .issue_by_number(&repo.id, 1, false)
+        .await
+        .unwrap()
+        .unwrap();
+    let labels = state.store.issue_labels(&issue.id).await.unwrap();
+    assert_eq!(labels.len(), 1, "label applied on create");
+    assert_eq!(labels[0].name, "bug");
+}
 #[tokio::test]