Skip to content

Trac: Modernize unsaved changes warning - #1000

Open
timse201 wants to merge 4 commits into
WordPress:trunkfrom
timse201:patch-23
Open

timse201 wants to merge 4 commits into
WordPress:trunkfrom
timse201:patch-23

Conversation

@timse201

@timse201 timse201 commented Oct 3, 2026 •

Copy link
Copy Markdown

Trac: Modernize unsaved changes warning on ticket forms.

  • Fix false warnings when submitting comments or tickets via keyboard shortcuts.
  • Fix unsaved warning protection being permanently lost if form submission is cancelled or prevented.
  • Prevent false warnings on whitespace-only comments.
  • Implement isDirty() check against defaultValue to prevent false warnings on pre-filled follow-up tickets.
  • Use standard window.addEventListener( 'beforeunload', ... ) instead of assigning directly to window.onbeforeunload.
  • Use event.preventDefault() as modern browsers no longer display custom dialog strings.
  • Track form submissions with an isSubmitting flag on #propertyform submit, resetting the flag if submission is prevented.

Summary by CodeRabbit

Summary

  • Bug Fixes
    • The ticket form’s unsaved-changes warning now ignores whitespace-only edits and detects changes to summaries, descriptions, and comments on existing tickets. It allows navigation when there are no unsaved changes and avoids prompting during submission. If submission is prevented, the warning is restored so unsaved edits remain protected.

Trac: Modernize unsaved changes warning on ticket forms.

- Fix false warnings when submitting comments or tickets via keyboard shortcuts.
- Fix unsaved warning protection being permanently lost if form submission is cancelled or prevented.
- Use standard `window.addEventListener( 'beforeunload', ... )` instead of assigning directly to `window.onbeforeunload`.
- Use `event.preventDefault()` and set `event.returnValue = ''`, as modern browsers no longer display custom dialog strings.
- Track form submissions with an `isSubmitting` flag on `#propertyform` submit, resetting the flag if submission is prevented.
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props timse201.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 00013e2f-e0f8-4fb7-834d-93c8dd6c1d47
📥 Commits

Reviewing files that changed from the base of the PR and between 4c962a2 and 2c42d2f.

📒 Files selected for processing (1)
  • wordpress.org/public_html/style/trac/wp-trac.js

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The ticket form warning compares trimmed summary and description values with their initial values. On existing tickets, a nonblank trimmed comment also counts as unsaved content. Submission suppresses the warning unless the browser prevents submission.

Changes

Ticket Form Unload Warning

Layer / File(s) Summary
Unload warning and submission state
wordpress.org/public_html/style/trac/wp-trac.js
The beforeunload listener checks trimmed summary and description values against their initial values. It also checks for a nonblank comment on existing tickets. Submitting #propertyform suppresses the warning. If submission is prevented, the handler clears the submission flag on the next event-loop turn.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: obenland

Merge Risk: ⚪ Minimal · up to 2c42d

No concrete merge-blocking behavior is established; the change appears mergeable with normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 2c42d

The change affects 1 system.

Changed systems: wordpress.org

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — wordpress.org (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in wordpress.org/public_html/style/trac/wp-trac.js: Adds a submission-state flag and isDirty, which returns true only when the selected field exists and its trimmed value differs from its trimmed initial value.
  • observed — Modified behavior in wordpress.org/public_html/style/trac/wp-trac.js: Replaces the single window.onbeforeunload handler with an event listener. Navigation is allowed while submitting or when no unsaved content is detected. New tickets are checked for modified summary or description; existing tickets are checked for those fields or a nonempty trimmed comment. When unsaved content remains, the event’s default is prevented. Unlike the removed handler, this does not return a warning string.
  • observed — Modified behavior in wordpress.org/public_html/style/trac/wp-trac.js: Adds a property-form submit handler that marks submission in progress and schedules a reset for the next tick if the submit event was prevented. This replaces clearing the unload handler on clicks to inputs within .buttons.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: modernizing the unsaved-changes warning on Trac ticket forms.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

- Prevent false warnings on whitespace-only comments.
- Implement `isDirty()` check against `defaultValue` to prevent false warnings on pre-filled follow-up tickets.
@timse201 timse201 changed the title Trac: Modernize unsaved changes warning on ticket forms Trac: Modernize unsaved changes warning Oct 3, 2026
@timse201 timse201 added [Type] Enhancement New feature or request [Site] Trac labels Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Site] Trac [Type] Enhancement New feature or request

Projects

Status: 👀 In review (PRs only)

Development

Successfully merging this pull request may close these issues.

1 participant