Add disable javascript option#723
Open
Pablo1Gustavo wants to merge 2 commits intochrome-php:1.15from
Open
Conversation
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
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.
Add
disableJavascriptoptionMotivation
The library already exposes
Page::setScriptExecution(bool)for toggling JavaScript on a per-page basis, but it must be invoked manually on every page after creation and before navigating, which is easy to forget and verbose when the caller's intent is "this whole browser instance never needs JavaScript".This PR introduces a first-class browser-level option,
disableJavascript, that disables JavaScript execution automatically on every page produced by the browser. It follows the same negative-boolean convention already used bynoSandboxanddisableNotifications(defaultfalse, active whentrue), so the public API stays consistent and predictable.Benefits
setScriptExecution(false)on every newly created page; the option is set once on the factory and propagates to all pages.laravel-pdf(which uses chrome-php under the hood) to render server-side reports. The HTML templates are fully static — no client-side rendering, no analytics, no third-party widgets — but bundled CSS frameworks still ship small JS payloads that fire on load and trigger reflows, network fetches, and timers. None of that affects the final PDF, but it can introduce flakiness — for example, a hanging promise delayingwaitForNavigation, or a timer keeping the page in a non-idle state. WithdisableJavascript => true, Chrome skips script execution entirely, making the render more deterministic and predictable. The performance impact depends entirely on what JavaScript the page contains.Implementation
The option flows from the factory down to each newly created page:
BrowserFactory—disableJavascriptis documented in the options docblock alongside the other negative-boolean options.BrowserProcess::start()— after the browser instance is created, if the option istruethe process callssetPageScriptExecutionDisabled(true)on the browser. The CLI approach (--blink-settingsor any command-line flag) is intentionally not used.Browser— exposes a new settersetPageScriptExecutionDisabled(bool)that toggles a protectedpageScriptExecutionDisabledflag. This mirrors the existingsetPagePreScript()pattern already used to inject behaviour on every new page.Browser::getPage()— when a brand-new page is created, afterPage.enable/Network.enable/Runtime.enable/Page.setLifecycleEventsEnabledand before the page is returned to the caller, the flag is checked andPage::setScriptExecution(false)is invoked (withawait()). The CDP method used isEmulation.setScriptExecutionDisabledwithvalue: true.The default value is
false, so existing code that does not passdisableJavascriptkeeps the previous behaviour exactly — JavaScript remains enabled and no extra CDP message is sent. Zero regression.The
README.mdoptions table has a new row documenting the option, following the formatting of the adjacentdisableNotificationsrow.Tests
Two new integration tests were added in
tests/BrowserFactoryTest.php, exercising a real Chrome instance against the existingtests/resources/static-web/javascript.htmlfixture (the same fixture used byPageTest::testSetScriptExecution):testDisableJavascriptOption()— creates a browser withdisableJavascript => true, navigates to the fixture, and asserts that the rendered body text is"javascript disabled"(the<noscript>content), proving that the inline<script>was not executed.testDisableJavascriptOptionDefault()— creates a browser without the option and asserts that the rendered body text is"javascript enabled", proving that the default behaviour is unchanged.Both new tests pass. The full test suite was also re-run and the only failures are three pre-existing flaky tests unrelated to this change (
BrowsingTest::testGetPagesClose,DomTest::testRootNodeIdIsUpdatedAfterReload,BrowserFactoryTest::testConnectToBrowser) — they fail in isolation on the base branch as well, with no involvement ofdisableJavascript.Manual testing
The change was also exercised visually with a small headed-mode demo (not committed) that opens a real Chrome window pointing to a page which:
<noscript>red banner saying "JavaScript is DISABLED",<script>that unhides a green "ENABLED" banner, starts a setInterval-driven counter, and wires a button toalert().Running the demo with
disableJavascript => falseshows the green banner, the counter ticking, and the button firing the alert. Running withdisableJavascript => trueshows only the red<noscript>banner, with no counter and no working button — confirming end-to-end that the CDP toggle is taking effect from the very first paint, with no leakage from the inline script.Public API surface
BrowserFactory— one new option key documented in the options docblock and in the README table. No new methods.Browser— one new public methodsetPageScriptExecutionDisabled(bool $disabled = true): void, symmetric with the existingsetPagePreScript(). This is additive only.