Sync master from upstream - #142
Merged
Merged
Conversation
The two toggles for sureforms-pro#1429. They live here because the page break settings component is shared and already mounted in both the Form Options panel and the page break block inspector, so adding them once reaches both. The form tag carries the result rather than Pro's button renderer: save and resume replaces that whole container through the srfm_page_break_buttons_html filter and would drop the attributes. The form tag is rendered exactly once and is already how this runtime receives per-form settings. Nothing is emitted when the feature is off, so existing markup is unchanged. With Pro absent the meta is unregistered and the toggles read undefined as false, which is how the rest of that panel already behaves. Refs brainstormforce/sureforms-pro#1429
TemplatePicker dereferenced document.querySelector( 'html.wp-toolbar' ) with no null check. The wp-toolbar class is applied by core in _wp_admin_html_begin() only when is_admin_bar_showing() is true, which short-circuits to false whenever XMLRPC_REQUEST, DOING_AJAX or IFRAME_REQUEST is defined, or wp_is_json_request() is true - all of which another plugin can trigger during an admin page load. In that case the query returns null and the effect throws a TypeError. Bail out when the element is missing, and restore the previous paddingTop on unmount instead of leaving the override in place. Fixes #3071
…r-toolbar-guard fix: template picker threw when the wp-toolbar class was absent
trimTextToWords() split its argument straight away. Any block that carries a slug but no label attribute passes undefined into it: srfm/register and srfm/login in Pro declare slug and no label, so a form holding one of them threw inside the editor's render pass and took the whole editor down with "The editor has encountered an unexpected error". The form still worked on the front end but could never be opened again. Resolve the argument to a string before splitting, so the helper cannot throw whatever a block hands it, and pin it with a unit test. Reported in support ticket #1539567.
use_delete() was the one write path in Base that never called cache_reset(). use_insert() and use_update() both do, so a count or lookup already cached earlier in the same request kept answering with the deleted row still in it -- the per-request cache is keyed by md5(query) on a singleton table instance, so every reader in that request saw the stale answer. Same class of bug as aca3e1d, which added the reset to use_insert() after survey live results and smart tags came back one entry behind. Fixes #3068
The connect flow started a 500ms setInterval from inside a promise continuation, with the interval id and popup handle held in function locals nothing could reach afterwards. Three problems followed: - No unmount cleanup. Both mount sites render OttoKitPage conditionally under a parent that survives, so an orphaned poll kept calling that live parent's setters -- setSelectedTab would move the user's tab out from under them. - window.open runs with no user activation left, so browsers routinely block it and return null. Dereferencing .closed then threw on every tick, before the clearInterval that would have stopped it, leaving an unbounded admin-ajax poll for the life of the page. - The mount effect re-runs on four deps, so polls could stack. Track the interval and popup in a ref, tear both down on unmount, bail out with a "allow popups" message when window.open is blocked, drop any poll already in flight before starting another, and guard the setters that in-flight requests can reach after unmount. Fixes #3070
SelectForm called the async getFormMarkup() without awaiting it, so setForm() stored a pending Promise rather than the response. Nothing ever read it: Empty.js owns `form`, passes it back down as a prop SelectForm never destructures, and the Choose button uses only formId. Awaiting it would store a resolved value that is still never read, and would keep the round-trip it pays for. The block's editor preview is the iframe in components/Edit.js, not this endpoint, and Empty.js is the placeholder shown before a form is chosen -- there is nowhere for markup to go. Both the state and the iframe date to the same original import, so this was never wired to anything. The one live effect was a wasted sureforms/v1/generate-form-markup request on every click in the dropdown, each a full server-side render of the form via Generate_Form_Markup::get_form_markup(). Remove the fetch and the dead state instead of awaiting them. Fixes #3067
…he-reset fix: reset the per-request DB cache after use_delete()
…ext Shadow
Both controls registered window.addEventListener('click', ...) inside a
useLayoutEffect with no cleanup, so one listener accumulated on window
per mount -- and these controls remount on every block selection. Each
stale listener also runs two document.querySelector calls on every click
anywhere in the editor, so the cost is per-click CPU, not just memory.
The handlers were inline anonymous functions and could never have been
detached, so each is hoisted to a named const before the cleanup is
added.
Fixed in modules/gutenberg/src/components/, not the top-level
src/components/ copies named in the issue. Those top-level copies have
no importers anywhere -- free src/, sureforms-pro (whose @components
alias points at free src/components/), and modules/gutenberg (which
aliases @components to its own copies) -- and appear in no built bundle,
so their listeners never register. The live copies are the SRFM-to-UAG
forks under modules/gutenberg, imported by the advanced-heading, image,
separator and icon block settings.
modules/gutenberg/build/ is a committed artifact that CI never rebuilds
(deploy runs only the root build, and Gruntfile excludes
modules/gutenberg/src), so the bundle is rebuilt here. The rebuild is
faithful: removeEventListener occurrences go 22 -> 24 and blocks.js
grows 352 bytes, with the dependency list unchanged.
Also fixes the same missing cleanup in src/admin/components/Header.js.
That one does not accumulate -- Header mounts once per admin page -- but
it is the same one-line shape. Root assets/build/ is gitignored and
built by CI, so no artifact is committed for it.
Fixes #3069
Addresses review on #3144.
1. stopAuthPoll() closed the popup, and ran from the unmount cleanup, so
switching a settings tab or closing the form dialog while the user was
logging in killed their window. Confirmed force-ui renders its dialog
as `{ open && ... }`, so dialog close really is an unmount path. The
cleanup now clears the interval and resets the ref only; success and
timeout still own closing the popup.
3. isMountedRef is set back to true when the effect runs, so StrictMode's
double-invoke or Fast Refresh cannot leave every continuation bailing.
4. window.SureTriggersConfig is process-wide, not component state, so the
outer handler writes it before the mounted bail -- matching the poll's
success path.
5. Comments no longer point at integrations/index.js, which is not on the
branch, and the new comment describes the real reason the poll is
dropped first: callers can invoke this while one is in flight.
…d-markup-fetch fix: drop the dead form-markup fetch from the block's form picker
…leanup fix: clean up the OttoKit OAuth poll and stop it running on a null popup
…ned-block-label Fix editor crash when a block has no label
Rebuilds the onboarding wizard against the Figma "New Improvements Aug 2026" designs. Routing, REST calls, the AI auth handshake and the migration import are unchanged; this is a visual and copy rebuild plus one new conditional step. Steps - Welcome: feature carousel that cross-fades through the designed artwork. Free installs see four slides, Pro installs see all nine. - Connect / SureMail: designed hero animations, new copy. - Add-ons: replaces the twelve-feature checkbox list with four tabs, each showing a draggable free-vs-premium comparison. Free installs only. - Cache conflict (new): shown when a recognised caching plugin is active, naming the plugin and linking to its setup guide. - User details moves to the last step before Done; the progress bar is hidden on Done. Analytics - premiumFeatures.selectedFeatures is replaced by viewedTabs + upgradeClicked, and cacheConflictAcknowledged is added. admin/analytics.php maps these to viewed_premium_tabs, premium_upgrade_clicked and cache_conflict_acknowledged. cacheConflictAcknowledged stays null until the step renders, so "never shown" stays distinct from "shown and not acknowledged". Also - Fixes initiateAuth and the access-key handler sending X-WP-Nonce: undefined on the dashboard bundle, where template_picker_nonce is not localized. - Onboarding artwork is emitted as separate hashed files instead of being inlined as data URIs; with the three retired inline icons removed, the dashboard bundle drops from 1.40 MB to 1.11 MB. - Both Router branches now derive their onboarding routes from one list, so a step can no longer be registered in only one of them.
Addresses review on #3146.
1. BoxShadowControl had the identical uncleaned window click listener and
is live: mounted three times per block by the image and icon block
settings, so selecting either still added three listeners per
selection after the previous commit. Same fix, and it shares the
bundle rebuild.
Verified against origin/dev: addEventListener stays 27,
removeEventListener 22 -> 25, removeEventListener("click" 0 -> 3,
blocks.js +405 bytes, blocks.asset.php dependency array unchanged.
2. Collapsed the duplicated three-line comment in typography and
text-shadow to the rule it is stating.
Re-swept both trees afterwards. The remaining unbalanced listeners are
not live: responsive-icons is a module-level DOMContentLoaded bootstrap,
getImageHeightWidth attaches to a local Image() that is collected with
it, and addInitialAttr guards on a listOfParentBlock of uagb/faq,
buttons, icon-list, restaurant-menu, social-share, content-timeline,
tabs and how-to -- none of which this module ships (advanced-heading,
icon, image, separator), so that branch never runs.
…istener-cleanup fix: remove the editor click listeners on unmount in Typography and Text Shadow
The add-ons step used the neutral badge, a grey chip that sat back into the card. The design calls for the dark pill, which is what marks the feature as gated rather than just labelled. Force UI already ships it: variant="inverse" is bg-background-inverse text-text-inverse, #1F2937 on #FFFFFF, with the default pill shape. No custom classes.
The divider now follows the pointer while it is over the card, so the comparison plays as you move across it. Dragging a handle was a step nobody has to take to see what the step is showing them. Also drops the orange border that appeared around the card. It was the focus ring: the range input covers the whole card, so :focus-within fired on click as well as on Tab, and every mouse interaction left a border behind. Keyed to :has(:focus-visible) instead, which keeps it for the keyboard, where the input is invisible and the ring is the only thing showing where focus is. The input takes no pointer events now, so mouse and touch are driven only by the move handler and the two cannot compute slightly different positions and fight over the divider. Pointer events rather than mouse events, because touch has no hover but a dragged finger still emits pointermove.
Connect step, per the copy sheet. The heading above the feature list now says what connecting actually does and what you get for it, rather than "get started"; the three bullets drop the AI-does-it-for-you framing; and both buttons say what they do, so "Connect" is no longer a bare verb next to an account you have not heard of yet. Welcome step: new headline, subheading and trust row, and the primary button is "Get Started". The arrow stays the button's own ChevronRight rather than a literal arrow in the string, which a screen reader would read out and which would point the wrong way in RTL. The Entries slide is now "Manage Form Entries". Left alone deliberately: email-delivery.js carries the same "Connect your free account to get started." string, but the replacement names AI form generations, which that step has nothing to do with.
1 (High) — rotation no longer destroys keyboard focus. Each tick remounts BeforeAfterSlider, whose range input is the step's only focusable control, so a tick while it held focus dumped focus on <body>; pausing on pointer alone meant a keyboard user could never hold it for more than one tick. Rotation now also stops on focus (capture phase, since focus/blur do not bubble), permanently once a tab is picked -- which is what the comment already claimed -- and under prefers-reduced-motion. 2 — upgradeClicked starts null and moves to false when the step actually renders. The step is hidden on Pro, which used to report premium_upgrade_clicked='no' for people who never saw it, while viewed_premium_tabs was correctly absent for the same user. Same shape as cacheConflictAcknowledged, which already did this. isset() skips null, so no PHP change. 3 — the cache step has a forward path that does not acknowledge. Every route through it acknowledged, so 'no' only ever meant "abandoned here". The primary button now advances the wizard rather than leaving it for a docs tab with muted ghost text as the only way on. 4 — get_caching_plugin_doc_url() takes the placement as utm_medium. Both surfaces sent form_checks_notice, so wizard clicks were attributed to the dashboard notice, defeating the attribution the helper exists to provide. Default keeps the existing call site unchanged. 5 — images/onboarding is build-time source only; webpack re-emits every import into assets/build/images/, so 2.8 MB shipped twice. The rest of images/ has to keep shipping, since gutenberg-hooks serves field-previews/* directly. 6 — the coupon comes from COUPON_CODE instead of being spelled out in a translatable string, so changing the constant cannot leave the sentence lying. Drops "Selected features", stale from the deleted checkbox UI. 7 — analytics tests. The old blob carried no selectedFeatures key, so the assertions claiming the old mapping is gone passed against the old code too; it now carries one. Adds the null case (distinct from absent: they are different paths through isset()), the false case, and the slug allowlist. viewed_premium_tabs now intersects against the four known slugs rather than filtering on type. 8 — the cache step only downgrades the acknowledgement from null, so browser Back no longer wipes a recorded true. 9 — the three inline comments drop the guessed 2.13.0. readme.txt is left for the release PR. 10 — the analytics test invokes detect_state_events() on the singleton rather than constructing a second Analytics, which registered four callbacks that survived into later tests. 11 — the addon comparison lookup is guarded, so a tab added without its illustration shows an empty panel instead of white-screening the step. 12 — removed a stray console.log, the dead session-storage key, and corrected the import-forms comment to the post-reorder step order.
The cache-conflict step no longer offers a skip; acknowledging is the only way past it. "Skip for now" is the AI step's label, from the copy sheet, and having it on two consecutive steps made it read like a generic escape rather than a choice about connecting an account. That makes cacheConflictAcknowledged an abandonment signal rather than a measure of intent: 'yes' for everyone who finishes, 'no' only alongside exited_early. This is the second of the two resolutions the review offered for that property. Both the step and admin/analytics.php now say so where the value is produced and where it is written, so nobody later reads it as "how many people ignored the caching warning". The primary button still advances the wizard and "View full guide" stays beside the tips -- that part of the review stands.
Stripe_Helper::get_license_key() is reached from the wp_ajax_nopriv_ payment and subscription intent handlers. On a license-cache miss the Pro check makes two wp_remote_request calls with a 30 second timeout each, so an unauthenticated visitor waits on SureCart before their payment intent is created - and a single slow response pins the cached verdict for the whole site from an anonymous request. Thread an $allow_remote_check flag through is_pro_license_active() and get_license_key(), and pass false from both front-end handlers. The default is unchanged, so admin screens still get a freshly verified answer. Passing the extra argument is safe against older sureforms-pro builds, where PHP ignores it. Companion to brainstormforce/sureforms-pro#1499.
The browser tagged a required-field miss, an expired captcha, a declined card and a server rejection naming a field as type 'blocked'. 2.12.6 kept those out of the failure count but still wrote them to the log file, so they were the most common lines in real logs and were pasted into every support report about some other failure. sanitize_entry() now drops the type at the shape gate, so no caller can write one, and the four browser call sites no longer buffer or send them.
wp_send_json_*() ends in a bare `die;` unless wp_doing_ajax() is true, and the WP test suite only routes wp_die_handler - not wp_die_ajax_handler - to a throwing handler. The first test to reach a JSON responder therefore killed the whole run while PHPUnit still exited 0, so CI reported success on a truncated suite: 18 of 2229 tests, dying in Test_Admin::test_track_ai_widget_usage. wp_doing_ajax() is forced true only underneath wp_send_json(), not suite-wide - forcing it globally changes plugin behaviour and cost 3 extra failures. The filter is a closure rather than '__return_true' because several tests call remove_filter( 'wp_doing_ajax', '__return_true' ), which shares its callback id and would silently remove this one too. The full suite now runs: 2229 tests, 78 errors, 34 failures. Those counts match origin/dev exactly, so none of them come from this branch - they are debt the truncation has been hiding.
The new @runInSeparateProcess tests in test-stripe-helper.php re-enter this bootstrap in a child process, and the WP test bootstrap drops and recreates every table when it runs. Each isolated test therefore wiped the database out from under the parent run, failing 11 later DB-backed tests in Client_Logger, Form_Submit, Form_Views, Forms_Data and Updater_Callbacks for reasons that had nothing to do with them. The parent has already installed by this point, so children inherit WP_TESTS_SKIP_INSTALL and reuse the same tables. With this, the branch matches the origin/dev baseline exactly (78 errors, 34 failures) with the three new tests passing on top.
With the run no longer truncating, 78 errors and 34 failures were visible on
dev. All of them are now resolved. Nothing here changes behaviour except the
three product fixes below, each of which is a real defect the tests caught.
Product fixes
- ai-form-builder/field-mapping.php: count() ran before is_array(), so a
non-array `form_data` on a public REST endpoint was a PHP 8 TypeError rather
than the intended invalid_form_data error.
- ai-form-builder/ai-auth.php: json_decode( $request->get_body() ) passed null
for an empty body. On PHP 8.1+ the deprecation notice is emitted into the
response, so the JSON came back unparseable wherever display_errors is on.
- multilingual/string-collector.php: email notification bodies were read from
a `body` key, but _srfm_email_notification's sanitize callback stores
`email_body`. The message body has never been registered for translation.
Test infrastructure
- tests/includes/class-srfm-unit-test-case.php adds factory() to the polyfill
test case. Seven classes were written against self::factory() while
extending Yoast's TestCase, which accounted for 64 of the 78 errors.
- test-editor-nudge.php no longer define()s WP_ADMIN. A constant cannot be
undone, so once any of its tests ran, every later front-end test in the
suite believed it was in wp-admin. set_current_screen() already covers it.
- test-post-types.php clears the srfm meta sanitize filters before a test
re-registers them: register_post_meta() appends a filter per call and
filters chain, so a second pass re-sanitized the first pass's output.
- test-analytics.php resets did_action('shutdown') between tests; the first
test to fire shutdown by hand changed the behaviour under test for the rest.
- test-abilities-registrar.php asks wp_has_ability() before unregistering, so
the unconditional cleanup stops raising _doing_it_wrong().
- Editor Nudge tests mirror the nonce into $_REQUEST, which is what
check_ajax_referer() reads.
Stale expectations corrected against what the code actually does:
sanitize_text_field() removing a <script> element whole, esc_url_raw() keeping
mangled markup rather than emptying it, wp_kses_post() keeping inner text,
phone auto-country falling back to the configured default, an empty gateway
being claimed as Stripe, get_error_message() returning a payload array,
get_account_name() returning '', delete_payment_webhooks() returning a
WP_REST_Response, the restriction scheduling message, the image block heading
only existing in the overlay layout, and the forms/manage enum gaining 'draft'.
Suite: 2230 tests, 9434 assertions, 0 failures, 0 errors.
Self-review follow-up. The instant-form URL test checked only that no '<' and no 'script>' survived esc_url_raw(), which would keep passing if the sanitizer started letting something else through. Assert the whole value so a change to what survives has to be made deliberately.
The redesigned wizard shares the onboarding_completed event name with the old one, and after this PR the two emit different property sets. A v2 run on a Pro install with no caching plugin emits none of the new properties, so the two flows could not be told apart downstream. The wizard now writes onboardingV2: true into the blob, and admin/analytics.php maps it to onboarding_v2 as yes/no. Absence reports 'no' rather than nothing, because a blob from the old wizard or no blob at all is exactly the case the property identifies.
…into master-dev-2.12.7-post
Master Dev 2.12.7
Test_Client_Logger::tearDown() switched logging off after every test, and test_clear_client_log ended by writing srfm_enable_logs = false, neither restoring what was stored before. Every test class that ran afterwards saw logging off, so Client_Logger::record_failure() recorded nothing and the four Test_Form_Submit notification fault tests failed in the full suite while passing on their own. Both now snapshot srfm_general_settings_options and restore it.
…settings feat: add the auto-advance settings for multi-step forms
Free now exposes the srfm_hide_promotions filter (Helper::hide_promotions(), default false) at each promotional surface, a hide_promotions flag for the dashboard JS, and an srfm.settings.general.additionalSections slot on Settings > General. The setting, its storage and its toggle live in SureForms Pro, which answers the filter. Reverts the general-settings storage, abilities key and settings state the setting needed in free.
… setting The long description explaining how views and conversion tracking works has been removed from the General Settings toggle. Only the exclusion note (admin/editor views don't count) and the Learn More link remain. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sion-description fix: remove redundant description from Show views and conversion rate setting
Recent entries, AI Quick Draft and the Finish setting up checklist are not registered, and their assets are not enqueued, while srfm_hide_promotions is true.
…ree-mode feat: add Distraction Free mode to hide promotional content (Pro)
…to dev-nr-2.12.8
Dev to Next Release 2.12.8
Version Bump 2.12.8
Auto-generated by /i18n command on PR #3167
chore: update i18n translations
chore: 2.12.8 ships 25th September
…ogs and Analytics
…n Free; top-align the welcome card
…and Partial Entries
…le_logs; drop the unused updateGlobalSettings slot arg
…stead of relying on CI having no network
…ction-order feat: Distraction Free settings and dashboard updates
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.
Syncs
brainstormforce/sureforms@master(2.12.8) into the public mirror.0d08900a2..1d728fa6b(908 upstream commits; 84 are new to the mirror, the rest were already here with identical signatures)master, so this diff is computed againstpublic/masterand no internal files appear in it.Highlights
🤖 Generated with Claude Code