Sync master from upstream - #124
Merged
Merged
Conversation
Three changes to the same panel. Drops the four passing rows: "Form submissions are completing normally", the notification and integration equivalents, and "No caching plugin that needs configuring was found". A panel that mostly confirms nothing is wrong is one people learn to skip, and it took permanent sidebar space from the cards that do have something to say. On a healthy site get_action_items() now returns an empty array and nothing renders. Gates the whole surface on the Enable Logs toggle. That toggle is what feeds the fault counters, so with it off whatever is still standing in them describes the site as it was, not as it is, and a site owner who turned logging off has opted out of this surface. Both gates sit in get_action_items() and has_action_item_warnings(), the only two entry points: the React panel, the wp-admin banners and the promo stand-down all route through one or the other. has_action_item_warnings() returning false also lets the promo cards come back rather than leaving the sidebar with a permanent hole. Simplifies the notice copy and drops the em dashes. The success branch in the classic notice and the green icon in the panel are kept deliberately: srfm_action_items is public, so a third party can still contribute an item with that status and both surfaces should handle it rather than render something broken.
Views are not counted for anyone who can edit the site (Form_Views::should_track()), but their entries were, so the two halves of the rate described different populations. Testing your own form five times added five to the numerator and nothing to the denominator: a form with 3 admin tests and 10 real visits reported 40% instead of 10%, or tripped the entries-exceed-views guard and rendered a dash on a form that was working perfectly well. The numerator now excludes the same people, from a get_users( capability => edit_posts ) list cached for the request. A logged-in subscriber stays counted on both halves, because their view is counted too. The Entries column is untouched and remains a true all-time count of every entry received. Two things this needed. NOT IN was not in the query builder's operator allowlist, so the clause would have been dropped silently. Added as a fallthrough on the existing IN case; no caller passed it before, since it would have been rejected, so nothing else changes. An empty list now emits the constant the empty set means rather than "col IN ()", which is a syntax error that fails the whole query. calculate_form_metrics() no longer takes an all-time entry count to reuse for forms created inside the tracking window. That count is unfiltered, so on a newly built form -- the one an admin has just been testing, which is exactly the case that skews -- it would have handed back the unexcluded number and undone the exclusion. It now takes only a form id, which also makes it impossible for the column and its sort order to be computed from different arguments. Also corrects a claim in that docblock: more entries than views was described as not possible in reality. An admin testing their own form is how it happened.
Master to Dev 2.12.6
Dev to Next Release 2.12.6
Direct coverage for the query-builder change: NOT IN survives the operator allowlist, and an empty list collapses to the constant the empty set means rather than emitting "col IN ()". The first of those is the one that matters. Without NOT IN on the allowlist the condition is dropped silently and the query matches every row, so an exclusion built on it fails open with no error anywhere. test_cache_reset() is what the coverage gate actually asked for. It attributes each changed line to the last public/protected function declared above it, and the @SInCE tag added to prepare_where_clauses' docblock sits above that function's own declaration, so it was booked against cache_reset(). The method genuinely had no test, so it has one now rather than a bypass label.
The help text only named who is left out, which reads as though being logged in is itself disqualifying. Subscribers and customers are counted on both halves, and on a membership or WooCommerce site that is most of the audience, so the omission was the misleading half. Verified against the gate rather than assumed: administrator, editor, author and contributor all have edit_posts and are excluded; subscriber and customer do not and are counted.
Everything above the rule configures how forms behave. Anonymous Analytics and Logs are about SureForms itself -- what it may report back, and what it records for support -- so reading them as more form settings is the wrong frame. A single rule is enough to say that without adding a heading or a second panel. Uses the border utilities the rest of the admin uses rather than a new component: force-ui ships no separator, and Navigation.js and ImportResult.js already draw rules this way. The border-0 and border-solid resets are load-bearing in wp-admin, which zeroes border styles out from under Tailwind.
Analytics is the only setting on the page that sends anything outward, and the least likely to be looked for, so it sits at the bottom. Logs moves up next to the rule, where someone sent here by support will find it without scrolling past an opt-in they did not come for. Order only. Both sections are unchanged.
The rule sat above Logs, which grouped Logs with Analytics and implied the two belong together. They do not: Logs is something SureForms does for this site, and Analytics is the one setting that sends anything outward. The rule now sits directly above Analytics and separates only that.
…517643) strip_unknown_field_keys() (added 2.12.3) removes any submitted `-lbl-` key whose block_id is not in the allowlist walked from the form's CURRENT post_content. But the block_id in the HTML a visitor actually submitted can differ from the current one — full-page caching (LiteSpeed) serving a stale render, or an editor rebuild reassigning block_ids. The result was silent, unrecoverable loss of real submission data: the keys were absent from the entry, no error, success message still shown. 2.12.2 has no such step, which is why rolling back fixed it. Fix: match on block_id OR slug. The slug is the stable field identifier (unlike block_id it is not regenerated on rebuild and is unaffected by a stale cached render), so a legitimately-submitted field whose block_id drifted now survives on its slug. A key with neither a known block_id nor a known slug is still foreign and dropped, so the anti-injection guard the function exists for is preserved. - Collect field slugs alongside block_ids in one walk (walk_field_identifiers). - Add get_known_field_slugs() + srfm_known_field_slugs filter, mirroring the block-id getter. - Keep a submitted key when its block_id OR slug matches; only strip when both are foreign. Applies to repeater child keys too. Regression tests: a drifted-block_id-but-known-slug field survives (revert-tested: fails without the fix), and a fully-foreign key is still dropped.
…cket Three related changes to what the failure notices point at. They share admin/admin.php and its test file, so they land together. Caching doc per plugin. Six of the twenty recognised caching plugins have a setup guide of their own, and the notice pointed everyone at the general page, so a site running WP Rocket got advice it had to translate first. Name and doc slug now live in one array rather than two keyed on each other, because kept apart they drift and the failure is silent: the link keeps working, it just degrades to the general page. Anything without its own guide, and a site where detection finds nothing, still lands on the general one rather than an empty href. A guide for notification failures. Email is the one failure here a site owner can usually fix without us, since it is almost always SMTP not being configured, so waiting on a support reply is the wrong default. The notice now offers the email troubleshooting guide next to Contact Support. It is optional keys rather than a new item shape: absent guide_label/guide_url render nothing, so the other categories need no branch in either renderer and neither does anything contributed through srfm_action_items. Contact Support keeps its acknowledgement behaviour -- reading a guide does not retire the notice, because nothing is fixed yet. The support email described the wrong problem. Subject and opening line were hardcoded to submissions, so a site whose notification email was broken raised a ticket titled "form submissions are failing" and got routed to the wrong queue. The count was wrong too: get_support_message() read get_fault_streak(), which returns the submission count only, so a notification failure quoted a number from a different counter, usually zero. Both now come from the category, out of one array so they cannot drift apart, and the form title goes in the body since support asks for it first. An unknown or absent category gets neutral wording rather than the submission copy, which would state something that may not be true. All eight documentation URLs verified live (HTTP 200) rather than inferred from the slug pattern.
A single form page served with none of its stylesheets while Breakdance was active. The form markup was correct, so it read as a conditional enqueue problem, but the assets were enqueued fine and simply never reached the browser. Breakdance filters template_include at priority 1000000, renders the theme template into an output buffer, and substitutes its own asset placeholders into the result. SureForms filters the same hook at PHP_INT_MAX, so on a single form it returns the Instant Form template and that buffered page is discarded. The work behind it is not: wp_head() has already run once inside the buffer, so every stylesheet is recorded in WP_Styles::$done, and the second wp_head() in the form template prints none of them again. Measured on a local reproduction: wp_head fired twice, 28 stylesheets recorded as printed, zero in the delivered page, and Breakdance's unreplaced <!-- BREAKDANCE_HEADER_DEPENDENCIES --> left in the head. After the fix, wp_head fires once and 20 stylesheets arrive, SureForms' own form.css, single.css and common.css among them. Standing Breakdance's filter down on these pages costs nothing that works today, because its output was already being thrown away. It removes the wasted render, leaves wp_head firing once, and lets the styles print normally. Scoped tightly on purpose. Breakdance is how the rest of these sites are built, so the guard is is_singular( SRFM_FORMS_POST_TYPE ) and the hook is `wp` -- the last point before template_include is applied and the first at which the queried object is known. It is also guarded on that exact filter being registered rather than on a version constant, since the removal is only correct while that callback is in the chain. The priority is matched exactly because remove_filter() needs it to find the entry. A mismatch there is a silent no-op: the page renders unstyled with nothing in any log, which is covered by its own test.
Someone who can fix a problem themselves should see that before they are pointed at a support queue. Email is almost always SMTP not being configured, so the guide is the better first move and Contact Support is the fallback. Emphasis follows position rather than identity. Reordering alone would have left the secondary styling on the leading action and the primary button on the one below it, so both renderers now key the emphasis off the order: whichever action leads is primary, and an item with no guide still leads with Contact Support looking exactly as it does today. The React panel builds an ordered list rather than two conditional blocks, which is what makes that rule expressible once instead of per button.
The two sit in the same sidebar column and were built differently: Quick Access nests a grey well inside its white card and puts white rows in it, while Form Checks laid bordered rows straight onto the card. Side by side they read as two components from different plugins. Form Checks now uses the same nesting -- bg-background-secondary well, white rows with shadow-sm-blur-1 and no border -- so the column is one thing. Rows only. The outer card, the collapse control and the row contents are untouched.
A mailto goes to whatever the machine has registered as its mail handler. On a machine with none configured -- common enough for someone who works in webmail -- clicking it opens nothing at all, and the person is left with a button that appears broken at the moment they are trying to report something broken. The compose window always appears now. The trade is real and worth stating: someone who does not use Gmail on the web gets a compose window for an account they may not want. They can still copy the message out of it, which beats a link that does nothing. Gmail truncates a long body silently, and the overflow is the end of the log -- the newest entries, the ones describing the failure being reported. So the URL is held to a 2000 character budget and the log is trimmed here instead, oldest first, with a line in the body saying it was shortened and where to get the whole file. Measured: 662 characters with an empty log, 1258 with a full one. No /u/0/ in the endpoint. That pins the first signed-in account, which on a machine with several is often the wrong one; without it Gmail composes from whichever account is active. The support address goes through a new srfm_support_email_address filter, so a reseller can point it at their own inbox rather than ours. Renamed from get_support_mailto_url, which would now describe the wrong thing. The mailto check in both renderers stays: srfm_action_items is public, so a third party can still contribute one, and that must open the mail client rather than a browser tab.
Four faults at once pushed the actual page below the fold on every admin screen, so SureForms' notices became the page. They now step through one at a time, with the count and arrows pinned to the top right of whichever notice is being read. Built in the browser rather than printed by PHP, which is the part worth keeping if this is ever changed: with JavaScript off every notice stays visible exactly as before. A control that cannot run must not be the thing that hides a warning. The controls are moved between notices rather than duplicated, so the count always sits with the message it counts. Positioned absolutely instead of floated so they cannot reflow the text, with padding reserved on that side -- a long form title would otherwise run underneath them. The counter is aria-live, since stepping swaps the text above it with no other signal, and it wraps at both ends so a run can be read round without hunting for the end. Engages at two notices. One needs no chrome, and the unrelated notices other plugins print are left alone -- only the action-item notices carry the class the carousel looks for.
Contact Support sent people straight into a composed message without ever showing them what it contained. Someone reporting a fault on their own site is entitled to read the diagnostics and the log first, and a support agent gets a clean paste instead of a screenshot of a notice. The action is now View details. It opens the same text that used to be posted blind, with Copy details and a Contact Support button pointing at sureforms.com/contact -- a form, which collects the licence and site details support would otherwise have to ask for, by which time the diagnostics are already on the clipboard. get_support_log_block() is extracted so the modal and the support URL share one source. They were built separately, which is how a "details" view drifts from what it claims to show. The wp-admin notice carries the text in the page, hidden beside it, rather than fetching it: a modal that has to make a request can fail at the exact moment someone is trying to report a failure. Written with esc_html and read as textContent, never parsed as HTML -- a log holds whatever a server put in an error message. The href stays as the no-JS path. It goes to the dashboard, where the same details are readable, and the click is only swallowed when the modal actually opens. Analytics gained view_details and copy_details per category, so neither click reports nothing. INCOMPLETE: the modal does not open in either surface. The click handler fires and the navigation is suppressed, so the overlay is being built and then not displayed. Ruled out: force-ui Dialog (a plain overlay fails identically), a stale cached script (forced a refetch), and JS errors (console is clean). The next thing to check is whether the overlay is in the DOM after the click and what is hiding it -- most likely the notice carousel's wrapper or WordPress's own .notice styling.
The body arrived in Gmail's compose window as one paragraph. The diagnostics and the log ran together, which is the opposite of the point: support has to be able to read it at a glance. Cause was CRLF. That is the mailto convention and correct when a mail client parses the URL, but Gmail's compose does not honour the carriage returns, so every break was dropped. Nothing parses this as a mail header any more -- the mailto path is gone -- so CRLF had no consumer left. Now bare LF throughout, which Gmail keeps: 22 encoded newlines in the URL and a 23-line body where there was previously one run of text. The log's own newlines were already LF, so the rewrite that normalised them is gone too. Covered by a test asserting the URL carries no %0D%0A and the body no carriage return, with a floor on the line count -- one long line is the failure being fixed, and it would otherwise look like a pass.
# Conflicts: # admin/admin.php
The diagnostics arrived in Gmail as one paragraph. Every line break was gone -- the site details and the log ran together, which is the opposite of the point. The URL was not the problem to solve. Gmail's compose window is a rich-text field: it drops newlines out of plain text whatever the URL encodes, and it escapes any markup passed through the body parameter, so no encoding survives that route. Sending the diagnostics through it was always going to lose the formatting. Copy details now writes both clipboard flavours. A rich composer takes the text/html and keeps every break, with <pre> holding the log's columns in line; a plain-text field takes the text/plain unchanged. Where ClipboardItem is unavailable it falls back to writeText, which loses the formatting but still beats copying nothing. The HTML is escaped in the browser, once, in the same place the markup is built. The source is a log holding whatever a server put into an error message, so it is never trusted as markup. Both surfaces share the behaviour: the wp-admin modal and the dashboard panel escape and wrap identically.
Three underlined links stacked in a narrow sidebar column read as a block of noise rather than as three actions. Underlined on hover only now, which is what Quick Access directly below already does -- the two panels were styling the same kind of control two different ways. Applies to Ignore as well: it is the same sort of control and was the odd one out either way.
# Conflicts: # src/admin/dashboard/ActionItems.js
View details opened the dashboard in a new tab -- the same page the reader was already on. Passing onClick through force-ui's Button did not stop it applying tag and href, so the click navigated instead of opening anything. It is a native button now. A control that opens a panel in place should not be an anchor: there is nowhere to navigate to, so there is no href to follow and nothing to open in a new tab. The other actions are real links and keep the Button they had. Also drops the underline from the actions, hover only, matching Quick Access directly below -- three underlined links stacked in a narrow column read as a block of noise rather than as three actions. Still open: the dialog does not render on click. The navigation is gone, which was the reported bug, but the overlay does not appear. Next step is to establish whether it reaches the DOM at all.
page_template() runs on template_include at PHP_INT_MAX and returns the Instant Form template whatever earlier filters returned. Any page builder that renders the whole page inside its own template_include filter has therefore already run wp_head()/wp_footer() into a buffer this discards, leaving every handle in WP_Styles::$done and WP_Scripts::$done -- so the Instant Form template's own head and footer print nothing. Clear both done lists when a head pass has already fired, rather than standing one named builder's filter down. Same bug, one control, and it covers Oxygen, Bricks, Divi and Cornerstone rather than only Breakdance. Also fixes the scope of the bug as recorded: srfm-form-submit is a footer script, so it was lost too and the form could not submit at all.
It is a button, not a link in prose. Rendering it as an anchor so it can carry an href is what brought the underline with it, and wp-admin's own anchor styles reach inside the component -- so the class has to say no on both states, not just the default one.
The support form asks for the diagnostics. Someone who reaches it without having copied them describes the failure from memory, which is the situation this whole flow exists to avoid. Copy details unlocks it. The unlock is its own state rather than the `copied` flag, which reverts after a couple of seconds so the button stops claiming "Copied" -- but having the clipboard stays true after the label has gone back. Only a successful copy unlocks it. A refused clipboard leaves the form locked, because the alternative is arriving there with nothing to paste and no idea why. While locked it is not an anchor. `disabled` on an <a> does nothing at all -- it still navigates -- so the href only exists once the copy has been made, and the locked state is a real disabled button with a title saying what to do. Opening a different failure resets it: each one is its own report, so the copy has to be made again for that one.
Round 4 asked for an explicit decision on the one remaining High: the diagnostics text was shipped to every admin's browser on every admin screen whether or not anyone ever opened the modal -- in srfm_admin's localisation JSON on the dashboard, and in a hidden div beside each classic notice. That text is written through the client error log, which is filled by a public REST route gated on a submit token any visitor can obtain from a form page rather than on a capability. So it is attacker-authored, and broadcasting it site-wide made any future escaping slip a manage_options-context problem instead of a one-admin one. Now served by Admin::handle_action_item_details() -- capability, then nonce, then the category -- and reached only by the admin who opens the dialog. The category needs no separate allowlist: get_open_failures() only ever returns keys in Client_Logger::CATEGORIES, so an absent category, an unrecognised one and a recognised one with nothing currently wrong all land on the same refusal. A non-string arrives as '' because sanitize_key() flattens a non-scalar (formatting.php:2194), which lands there too. Both dialogs open immediately with a placeholder and fill in when the response arrives. Round 1's concern still stands -- a modal that has to make a request can fail at the exact moment someone is trying to report a failure -- so a failed fetch says so in place, drops the Copy button (an error message on the clipboard is not a report), and unlocks Contact Support against an untagged troubleshooting-form URL. These notices are not dismissible and Contact Support is the only action that retires them, so a failed fetch must not take the way out with it. Verified on sureform-3.test, both surfaces and both paths: the page source no longer contains the log text, the placeholder is replaced by the 21-line report, Copy unlocks Contact Support, and a bad nonce produces the failure state with a working Contact Support. Return-focus lands back on the trigger on both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3102 and #3103 landed on dev since this branch forked. One conflict, in Client_Logger::get_tail()'s memo-key comment: both sides describe the same fix (41143db80, authored on #3102 and cherry-picked here), dev's wording kept. Verified after resolving: PHPStan clean, PHPCS clean, Test_Client_Logger 59/59, the action-item and details tests 25/25, ESLint clean, Jest 66/66, and the bundle rebuilds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-keys-data-loss Fix: stop silently dropping submitted fields when block_id drifts (#1517643)
**The success path could hand back less than the failure path.** `.catch()` fell back to the localised support URL; the success path was `json.data.support_url || ''`. Fixed at the source rather than at the two call sites: `get_support_contact_url()` now never returns ''. Two things were wrong there. `esc_url_raw( $url, [ 'http', 'https' ] )` dropped `mailto:`, which is a legitimate destination for a white-label support contact and which `get_action_items()` already allows on the sibling item URLs -- so `mailto` is now in the allowlist. And any filter value that still cannot survive escaping falls back to the unfiltered SureForms URL, which is built here rather than supplied and so always escapes. These notices are `dismissible => false` and Contact Support is the only action that retires them, so '' there is an undismissable notice with nothing on it that works. Both clients also fall back to the localised URL on success, so neither depends on a server invariant it cannot see, and both drop `target="_blank"` for a `mailto:` -- it has no document to open and would leave a blank tab behind. **safeUrl() was checking a spelling the DOM never sees.** A browser trims C0 controls and spaces off both ends and strips tab, LF and CR from anywhere in the string before parsing the scheme, so " javascript:" and "java\tscript:" both failed to match `^[a-z]` and were returned untouched. It now applies that normalisation first and returns the normalised string rather than the input -- validating one spelling and handing back another is how a scheme check gets walked past. Protocol-relative "//" and its "\\" spelling are rejected too: they carry no scheme to check and resolve to another origin, so the schemeless-is-relative branch must not claim them. 12 new Jest cases, all failing on revert. **Smaller:** the vanilla dialog's setters now no-op once their overlay is detached; `aria-busy` is on the dashboard's diagnostics region as well as the classic one; `Previously reported:` is wrapped in __() -- its value already goes through wp_date(), so the line was mixed-locale either way. POT regeneration deliberately excluded, per Vansh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The committed sureforms.pot arrived here through the #3102 merges and is the
output of a `wp i18n make-pot` run that died part-way:
Fatal error: Allowed memory size of 134217728 bytes exhausted
in .../vendor/mck89/peast/lib/Peast/Syntax/Scanner.php on line 604
Peast is the JS parser make-pot uses. The process is killed mid-scan, so the
file is written short with no error in it -- it stopped referencing
admin/admin.php at line 3430 of 5249, dropping 74 translatable calls. dev's
copy reaches 4746 of 4782 and is essentially whole, so merging this would have
lost the existing translations for every dropped msgid on the next import.
Restored to dev's version, so this PR touches no translations at all.
Regeneration belongs in its own change, and needs the memory limit raised:
php -d memory_limit=2G "$(command -v wp)" i18n make-pot . --skip-audit \
--exclude='...' languages/sureforms.pot
That completes, reaches line 5212, and picks up all the new strings. Without
it the next run produces another silently short file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
feat: replace Contact Support with View details and a diagnostics modal
…to dev-nr-2.12.7
Dev to Next Release 2.12.7
Also fixes the dead `replace:plugin_const` grunt task, whose regex still matched `UAGB_VER` from Ultimate Addons. `SRFM_VER` was never bumped by `grunt version-bump` — it has been hand-edited every release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Version Bump 2.12.7
- Add a short description to the Enable email summaries toggle - Shorten the Show views and conversion rate description - Remove em dash from Show views and conversion rate description Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace em dash with a comma in the feature bullet point copy for consistency with the rest of the UI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ult port Both from adi3890's review of #3129, where he scoped them to 2.12.8 rather than blocking the release. **walk_field_identifiers() ran twice per submission.** It returns ids and slugs together precisely so a form is parsed once, but get_known_field_block_ids() and get_known_field_slugs() each held their own `static $cache`, so parse_blocks() plus the recursive collect ran once for each -- on every submission, on markup that can be large. One shared memo in get_field_identifiers() now. Only the walk is memoised, unchanged: the filtered results stay uncached so a third party returning a malformed value cannot poison the set for the rest of the request. **The log scrubber truncated our own stack frames on any non-default port.** The URL pattern captures the whole authority, so a frame from a site on :8443 reads `example.test:8443`, while $site_host comes from PHP_URL_HOST and never carries a port. Every own frame failed the same-origin check and was cut to /[path] -- deleting the filename, the line and the column, which is the exact loss that exemption exists to prevent. Local installs, staging behind a proxy and anything off 80/443 were all affected. The port is stripped before comparing, trailing :digits only so an IPv6 literal keeps its brackets and its own colons. Same host on a different port is treated as ours: on a WordPress install that is the same site behind a dev server or a proxy, and the alternative is deleting the diagnosis. Both tests revert-tested. The scrubber one needed a home_url filter -- the existing provider builds its frames from home_url(), which has no port on the test site, so both sides of the comparison matched by accident and it could never have caught this. The walk one observes rather than counts: the content is removed between the two calls, so a second walk has nothing to find. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
From Vansh, reading a real debug log: a form with nothing wrong was raising "We noticed a form submission failure", and the log behind it was full of `After-submission step failed: TypeError: Load failed`. That step runs *after* the entry is saved -- the comment directly above the logging call already said "The submission itself succeeded" -- but it logged type 'error', which is_fault() counts unconditionally, and append() files every fault it is handed under 'submission'. So the notice read "Visitors may not be able to reach you, and their entries were not saved" about entries that were saved, to visitors who saw the success message, on the one notice that cannot be dismissed. "Load failed" is Safari's message for a fetch the browser abandoned, which is exactly what a redirect confirmation does to an in-flight request and exactly what the keepalive above it exists to survive. The browser noticing it lost the response says nothing about whether the server ran the work -- the endpoint is guarded by is_after_submission_process_triggered and usually has. Both after-submission paths now log type 'after_submission': - Without a status -- the browser-side abort -- that is never a fault. Still logged, so the diagnosis survives, the way 'blocked' already is. - With a 5xx or 403, the server said it failed, and that is a fault. Recorded against 'integration', not 'submission': srfm_after_submission_process is where integrations and webhooks hook in, so "a third party is not receiving them" is the true sentence. The category is derived server-side from the type -- the client names what happened, the server decides what it means. Three tests, mutation-tested rather than just revert-tested: treating after_submission as an unconditional fault fails the first, and filing it under 'submission' fails the second. The third pins 'blocked' staying silent, which is the precedent this follows and which nothing covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…atim
Vansh, reading a copied report: the prose translates but the diagnostic block's
labels did not, so a German site produced a half-translated document -- and
oddly, the *values* beside those labels were already translatable while the
labels themselves were bare English:
'Caching: ' . ( '' !== $caching ? $caching : __( 'none detected' ) )
Seven labels wrapped: Site, SureForms, SureForms Pro, WordPress, PHP, Caching,
Recorded failures. The line the site owner reads is copy and belongs in the
catalogue.
What deliberately does not move:
- The values beside them. A version, a URL, a plugin name are machine data.
- The debug log block. Its JSON is the raw record support greps, and rewriting
any of it would defeat the reason it is pasted.
Tested through a gettext filter rather than a real locale: the suite loads no
translations, so an untranslated string is indistinguishable from a translated
one that happens to match. The filter marks anything that reached the
catalogue, so a label that was never wrapped simply lacks the marker. Asserted
in both directions -- every label carries it, the JSON entry lines carry it
nowhere. Fails on revert.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolved src/admin/settings/pages/General.js in favour of next-release. That branch already reworked the views/conversion description into a wrapper span with two separate __() calls, and its comment explains why the long msgid is kept verbatim: all seven shipped locales translate it, so re-punctuating it would orphan every one of them. It also carries the editor-exclusion sentence added in #3102, which this branch predates. The short rewrite this branch carried would have reverted both, so it is dropped. The email summaries description is unaffected and stays.
Points at https://sureforms.com/docs/show-views-and-conversion-rate/ for the detail the inline copy deliberately does not carry. Added as a third span inside the existing wrapper, in its own __() call, so no shipped msgid changes and no locale loses a translation. Matches the anchor already used by the usage tracking setting below it.
From Vansh: with logging disabled nothing should be logged, and no Form Checks
or notices should appear.
Display was already right -- get_action_items() skips the first-party items and
has_action_item_warnings() returns false. The counter was not. record_failure()
had no enabled check, and the two call sites in form-submit.php sit directly
beside an append() that the check does stop, so on a site with logging switched
off the log file stayed empty while failure state kept accumulating in the
options table.
Measured before, logging off:
log_file_bytes 0
failures_option {"notification":{"count":1,"form_id":42,...}}
open_failures ["notification"]
The display gate hid that, which is what let it go unnoticed -- until logging
was switched back on, at which point every fault recorded during the quiet
period appeared as a notice, behind a View details report whose debug log is
empty because nothing had been written. A failure claim with no evidence
underneath it.
After: nothing written, nothing counted, and switching logging on surfaces no
backlog.
Gated inside record_failure() rather than at the three call sites, for the
reason append() already gives: the guard belongs on the function that writes,
not on today's callers. clear_category() and the acknowledge helpers stay
ungated -- they only remove state, and must keep working so the toggle strands
nothing.
The test asserts across the toggle rather than in one state. "No notice right
now" was already true, and testing only that is what would have missed this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dash fix: remove em dash from partial entries empty state text
…and-scrubber-port fix: three quiet wrongs — double block walk, scrubber port, and a false submission-failure notice
Auto-generated by /i18n command on PR #3129
chore: update i18n translations
From Vansh: "Rate SureForms 5 stars" rendered directly beneath "We noticed a
notification failure on Contact Form".
All three engagement notices -- rating, Getting Started and the Thank You prompt
-- already asked has_action_item_warnings() before showing. The problem was what
that answered. It re-stated its conditions rather than reading them, and the
restatement was narrower than the display: has_persistent_failures() reads the
`submission` counter alone, while the notices and the Form Checks panel warn on
any open failure in any of the three categories.
So a notification or integration failure left the gate false. Measured, one open
notification failure with the caching advice dismissed:
warning_notices_on_screen ["notification_error"]
has_action_item_warnings false
Submission was covered only incidentally, because FAULT_THRESHOLD is 1. Raise it
and submission would have joined them.
Derived from get_first_party_action_items() now -- the function that builds those
warnings -- so the gate cannot drift from the display again. Both constraints the
old version documented are kept: it still does not call get_action_items(), which
records an impression as a side effect and must never run from a show_if; and it
still reads the first-party set specifically, so an item contributed through
`srfm_action_items` cannot suppress notices unrelated to it. Verified
get_first_party_action_items() has no side effects of its own -- the impression
tracking sits one level up in get_action_items().
The test asserts the gate and get_action_items() together, because the defect was
the two disagreeing and checking the gate alone is what let it through. It covers
submission as well, so raising FAULT_THRESHOLD cannot quietly reopen this.
Pre-existing and untouched: the two Test_Thankyou_Prompt_Notice failures are
test-order interference on next-release itself, confirmed against a stash.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s-to-any-warning fix: the rating notice appeared alongside a failure warning
Release date moved from the 11th to the 12th, and the Patchstack disclosure
added to the 2.12.7 changelog:
* Fix: This update addressed a security bug. Props to nh4tvd from
Patchstack for reporting it responsibly to our team.
Worded and positioned to match 2.12.6's own props line -- last in the list,
after the Fix entries -- so the changelog reads consistently release to
release. This one names the individual reporter as well as Patchstack, which
2.12.6's did not.
README.md is generated from readme.txt by `grunt readme`, so it is regenerated
here rather than hand-edited. The regeneration introduced no churn beyond the
three changed lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-security-changelog chore: 2.12.7 ships 12th September, and carries the security props line
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Routine sync of
brainstormforce/sureforms@master(private) into this public mirror, with internal-only paths stripped.Range
295544f7(2.12.6)77468feb(2.12.7)The branch is capped with a merge commit whose first parent is
masterof this repo, so the diff below is computed against the mirror and shows only real upstream changes — no internal-file deletions.What's in it — 2.12.7
Strip
71 files removed across the documented internal paths:
.claude,.scripts/git-hooks,internal-docs, the wholedocs/tree, the analysis markdown files,tests/play/specs/TODO.md, five internal release workflows, and threebin/release scripts — plus all threeCLAUDE.mdfiles (root,inc/abilities/,tests/play/specs/).Post-strip verification:
git ls-files '*.md'returns onlyREADME.mdand the four pre-existing third-party/module readmes (inc/abilities/ABILITIES.md, twoinc/lib/**,modules/gutenberg/readme.md,tests/play/README.md).git ls-tree -ron the capped tree matches 0 internal paths.Signatures
All 826 commits re-signed; 0 unsigned. GitHub reports
verified=true, reason=validfor both the merge cap (343243690) and the signed tip (127ef25b6).Re-signing rewrites the committer to the signing identity so the registered key can produce verified signatures; the original author of every commit is preserved.