Skip to content

Release v3.1.0 - Critical Bug Fixes & Enhanced Robustness #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 24, 2025
Merged

Release v3.1.0 - Critical Bug Fixes & Enhanced Robustness #204

merged 1 commit into from
Oct 24, 2025

Conversation

@renatomarinho
Copy link
Collaborator

@renatomarinho renatomarinho commented Oct 24, 2025

🎉 Release v3.1.0 - Critical Bug Fixes & Enhanced Robustness

This release includes critical bug fixes and significantly improved test coverage to ensure package stability and reliability.

🐛 Critical Bug Fixes

1. JavaScript Comment Removal Bug (#180, #184)

Issue: The RemoveComments middleware was incorrectly removing // characters inside JavaScript strings, breaking code like:

var url = "http://example.com"; // Comment
// After processing: var url = "http:";

Fix:

  • ✅ Complete refactoring with character-by-character parser
  • ✅ Proper string state tracking (single quotes, double quotes, regex literals)
  • ✅ URL preservation (http://, https://, ://)
  • ✅ Line ending preservation for cross-platform compatibility

Tests: 8 new comprehensive tests covering edge cases

Fixes: #180, #184

2. InsertDNSPrefetch Extracting URLs from JSON-LD (#179)

Issue: The InsertDNSPrefetch middleware was incorrectly extracting URLs from:

  • JSON-LD structured data (<script type="application/ld+json">)
  • JavaScript code inside <script> tags
  • CSS code inside <style> tags

This caused unwanted DNS prefetch tags and SEO issues.

Fix:

  • ✅ Refactored to extract URLs ONLY from HTML resource attributes
  • ✅ Supports: <script src>, <link href>, <img src>, <a href>, <iframe src>, <video>, <audio>, <source>
  • ✅ Ignores: Script content, style content, JSON-LD, inline JavaScript

Tests: 3 new tests for JSON-LD, JavaScript, and edge cases

Fixes: #179

3. PCRE Error Handling for Large HTML

Issue: Large HTML documents could exceed PCRE backtrack/recursion limits, causing blank pages.

Fix:

  • ✅ Added PCRE error detection in PageSpeed::replace()
  • ✅ Graceful fallback to original content on PCRE errors
  • ✅ Prevents blank pages

Tests: 2 tests with 5,000 and 20,000 line HTML documents

Related: #184

📚 Documentation & Test Coverage

DeferJavascript Comprehensive Test Suite (#173)

Added: 17 extremely robust tests (47 assertions) documenting all edge cases:

✅ Basic Functionality:

  • Adds defer to external scripts
  • Preserves existing defer attributes
  • Respects data-pagespeed-no-defer
  • Does NOT modify inline scripts

✅ Edge Cases:

  • Type attributes (text/javascript, module, application/ld+json)
  • Async attribute
  • Integrity and crossorigin attributes
  • Multiple scripts with mixed configurations
  • Empty src attributes
  • Relative/absolute/protocol-relative paths
  • Single/double/no quotes
  • Performance: 100 scripts in <100ms

✅ GLightbox Scenario: Documents the exact issue from #173 with 4 solution options

Related: #173

🔧 Dependency Updates

  • ✅ orchestra/testbench → ^10.6.0
  • ✅ actions/checkout → v5
  • ✅ actions/cache → v4
  • ✅ actions/github-script → v8

📊 Test Results

🚀 Impact

Performance

  • ✅ No performance regressions
  • ✅ Large HTML processing verified (20,000 lines)
  • ✅ PCRE safeguards prevent blank pages

Stability

  • ✅ JavaScript comment removal no longer breaks code
  • ✅ DNS prefetch no longer interferes with SEO/structured data
  • ✅ Robust error handling for edge cases

Compatibility

  • ✅ Laravel 10, 11, 12
  • ✅ PHP 8.2, 8.3
  • ✅ All existing functionality preserved

📝 Breaking Changes

None - This is a bug fix release maintaining full backward compatibility.

🎯 Closed Issues

📦 Installation

composer require vinkius-labs/laravel-page-speed

Or for immediate access to these fixes:

{
    "require": {
        "vinkius-labs/laravel-page-speed": "dev-develop"
    }
}

👥 Contributors

@renatomarinho - All bug fixes and test coverage improvements

Ready to merge: All tests passing, no conflicts, fully backward compatible.

@renatomarinho
test: add extremely robust DeferJavascript test coverage
60c5fa9 
- Created 17 comprehensive tests with 47 assertions
- Documents GLightbox scenario from issue #173
- Tests all edge cases: type attributes, async, integrity, crossorigin
- Tests JSON-LD, module scripts, nomodule, empty src
- Tests relative/absolute paths, protocol-relative URLs
- Tests single/double quotes and no quotes
- Performance test: 100 scripts in <100ms
- All 62 tests passing with 246 assertions
- Relates to #173
Copilot AI review requested due to automatic review settings October 24, 2025 21:16
Copilot AI reviewed Oct 24, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This release focuses on critical bug fixes and comprehensive test coverage improvements for the Laravel PageSpeed package. The primary issues addressed include JavaScript comment removal breaking URLs, DNS prefetch extracting URLs from JSON-LD/script content, and PCRE errors with large HTML documents.

Key Changes:

  • Refactored RemoveComments middleware with character-by-character parsing to preserve URLs in JavaScript strings
  • Fixed InsertDNSPrefetch to only extract URLs from HTML resource attributes, not script/style content
  • Added PCRE error handling for large HTML documents to prevent blank pages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/Middleware/DeferJavascriptRobustTest.php
Comment on lines +10 to +13
protected function getMiddleware()
{
$this->middleware = new DeferJavascript();
}
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getMiddleware() method sets $this->middleware but does not return it, and the property is never used in any test. This method appears to be dead code that should be removed, or it should return the middleware instance if it's intended to be used.

Suggested change
protected function getMiddleware()
{
$this->middleware = new DeferJavascript();
}

Copilot uses AI. Check for mistakes.
tests/Middleware/DeferJavascriptRobustTest.php

// Should NOT modify inline script (no src attribute means no defer added)
$this->assertStringContainsString("console.log('inline');", $actual);
$this->assertStringNotContainsString("console.log('inline');" . ' defer', $actual);
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string concatenation \"console.log('inline');\" . ' defer' is unnecessarily complex. This should be simplified to a single string literal: \"console.log('inline'); defer\".

Suggested change
$this->assertStringNotContainsString("console.log('inline');" . ' defer', $actual);
$this->assertStringNotContainsString("console.log('inline'); defer", $actual);

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Oct 24, 2025

🔍 Pull Request Validation Results

  • ✅ Code Style: PSR-2 compliant
  • ✅ Tests: All passing
  • 📊 Coverage: 100.00%
  • 🚀 Ready to merge

Automated validation by GitHub Actions

@renatomarinho renatomarinho merged commit 4fedf5a into master Oct 24, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@renatomarinho