Skip to content

Commit 2cbec6d

Browse files
renatomarinhorenatomarinho
committed
fix: update InlineCss middleware to correctly handle framework-specific class attributes and add comprehensive test suite
1 parent 049852c commit 2cbec6d

File tree

3 files changed

+279
-2
lines changed

3 files changed

+279
-2
lines changed

‎CHANGELOG.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,24 @@
22

33
All notable changes to `laravel-page-speed` will be documented in this file.
44

5+
## [Unreleased]
6+
7+
### Fixed
8+
- 🐛 **InlineCss Middleware**: Fixed regex pattern to prevent matching framework-specific class attributes (Issues #75, #133, #154)
9+
- Changed from `/class="(.*?)"/` to `/(?<![-:])class="(.*?)"/i` using negative lookbehind
10+
- Now correctly ignores `ng-class` (AngularJS), `:class` (Alpine.js), `v-bind:class` (Vue.js)
11+
- Horizon dashboard now works correctly with InlineCss (Issue #133)
12+
- AngularJS applications with `ng-class` work correctly (Issue #75)
13+
- Alpine.js `:class` shorthand works correctly (Issue #154)
14+
- Vue.js `v-bind:class` works correctly
15+
16+
### Added
17+
- ✅ New test suite `InlineCssJavaScriptFrameworksTest` with 7 comprehensive tests (42 assertions)
18+
- ✅ Tests for AngularJS `ng-class` compatibility
19+
- ✅ Tests for Alpine.js `:class` shorthand compatibility
20+
- ✅ Tests for Vue.js `v-bind:class` compatibility
21+
- ✅ Tests for mixed framework scenarios
22+
523
## [3.0.0] - 2025-01-24
624

725
### ⚠️ BREAKING CHANGES

‎src/Middleware/InlineCss.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ private function fixHTML()
6969
$tmp = explode('<', $this->html);
7070

7171
$replaceClass = [
72-
'/class="(.*?)"/' => "",
72+
'/(?<![-:])class="(.*?)"/i' => "",
7373
];
7474

7575
foreach ($tmp as $value) {
76-
preg_match_all('/class="(.*?)"/', $value, $matches);
76+
preg_match_all('/(?<![-:])class="(.*?)"/i', $value, $matches);
7777

7878
if (count($matches[1]) > 1) {
7979
$replace = [

‎tests/Middleware/InlineCssJavaScriptFrameworksTest.php‎

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
<?php
2+
3+
namespace VinkiusLabs\LaravelPageSpeed\Test\Middleware;
4+
5+
use VinkiusLabs\LaravelPageSpeed\Middleware\InlineCss;
6+
use VinkiusLabs\LaravelPageSpeed\Test\TestCase;
7+
8+
class InlineCssJavaScriptFrameworksTest extends TestCase
9+
{
10+
protected function getMiddleware()
11+
{
12+
$this->middleware = new InlineCss();
13+
}
14+
15+
/**
16+
* Test Issue #75: AngularJS ng-class should not be affected
17+
*
18+
* The InlineCss middleware was matching all attributes containing "class",
19+
* including ng-class, which broke AngularJS applications.
20+
*/
21+
public function test_angularjs_ng_class_preserved(): void
22+
{
23+
$html = '<!DOCTYPE html>
24+
<html ng-app="myApp">
25+
<head></head>
26+
<body>
27+
<div ng-class="{active: isActive, disabled: isDisabled}" style="padding: 10px;">
28+
<p class="text-bold" style="font-weight: bold;">Regular class with style</p>
29+
<span ng-class="dynamicClass">Dynamic</span>
30+
</div>
31+
</body>
32+
</html>';
33+
34+
$middleware = new InlineCss();
35+
$result = $middleware->apply($html);
36+
37+
// ng-class attributes must be preserved exactly
38+
$this->assertStringContainsString('ng-class="{active: isActive, disabled: isDisabled}"', $result);
39+
$this->assertStringContainsString('ng-class="dynamicClass"', $result);
40+
41+
// Should have style tag with converted inline styles
42+
$this->assertStringContainsString('<style>', $result);
43+
$this->assertStringContainsString('page_speed_', $result);
44+
45+
// Inline styles should be converted to classes
46+
$this->assertStringNotContainsString('style="padding: 10px;"', $result);
47+
$this->assertStringNotContainsString('style="font-weight: bold;"', $result);
48+
}
49+
50+
/**
51+
* Test Issue #154: Alpine.js :class shorthand should not be affected
52+
*/
53+
public function test_alpinejs_class_shorthand_preserved(): void
54+
{
55+
$html = '<!DOCTYPE html>
56+
<html>
57+
<head></head>
58+
<body>
59+
<div x-data="{ open: false }">
60+
<button :class="{ \'bg-blue\': open, \'bg-gray\': !open }" style="padding: 5px;">Toggle</button>
61+
<div class="container" :class="open ? \'visible\' : \'hidden\'" style="margin: 10px;">
62+
<p style="color: red;">Content</p>
63+
</div>
64+
</div>
65+
</body>
66+
</html>';
67+
68+
$middleware = new InlineCss();
69+
$result = $middleware->apply($html);
70+
71+
// Alpine.js :class must be preserved
72+
$this->assertStringContainsString(':class="{ \'bg-blue\': open, \'bg-gray\': !open }"', $result);
73+
$this->assertStringContainsString(':class="open ? \'visible\' : \'hidden\'"', $result);
74+
75+
// Inline styles should be converted to classes
76+
$this->assertStringNotContainsString('style="padding: 5px;"', $result);
77+
$this->assertStringNotContainsString('style="margin: 10px;"', $result);
78+
$this->assertStringNotContainsString('style="color: red;"', $result);
79+
80+
// Should have page_speed classes
81+
$this->assertStringContainsString('page_speed_', $result);
82+
}
83+
84+
/**
85+
* Test: Vue.js v-bind:class should not be affected
86+
*/
87+
public function test_vuejs_vbind_class_preserved(): void
88+
{
89+
$html = '<!DOCTYPE html>
90+
<html>
91+
<head></head>
92+
<body>
93+
<div id="app">
94+
<div v-bind:class="{ active: isActive, \'text-danger\': hasError }" style="padding: 20px;">
95+
<p style="font-size: 14px;">Message</p>
96+
</div>
97+
<span v-bind:class="classObject">Object</span>
98+
<button v-bind:class="[activeClass, errorClass]" style="border: 1px solid;">Array</button>
99+
</div>
100+
</body>
101+
</html>';
102+
103+
$middleware = new InlineCss();
104+
$result = $middleware->apply($html);
105+
106+
// Vue.js v-bind:class must be preserved
107+
$this->assertStringContainsString('v-bind:class="{ active: isActive, \'text-danger\': hasError }"', $result);
108+
$this->assertStringContainsString('v-bind:class="classObject"', $result);
109+
$this->assertStringContainsString('v-bind:class="[activeClass, errorClass]"', $result);
110+
111+
// Inline styles should be converted
112+
$this->assertStringNotContainsString('style="padding: 20px;"', $result);
113+
$this->assertStringNotContainsString('style="font-size: 14px;"', $result);
114+
$this->assertStringNotContainsString('style="border: 1px solid;"', $result);
115+
}
116+
117+
/**
118+
* Test: Mixed usage of regular class and framework-specific class attributes
119+
*/
120+
public function test_mixed_class_and_framework_attributes(): void
121+
{
122+
$html = '<!DOCTYPE html>
123+
<html>
124+
<head></head>
125+
<body>
126+
<!-- AngularJS -->
127+
<div class="card" ng-class="{expanded: isExpanded}" style="border: 1px solid;">Angular</div>
128+
129+
<!-- Vue.js -->
130+
<div class="box" v-bind:class="vueClasses" style="padding: 10px;">Vue</div>
131+
132+
<!-- Alpine.js -->
133+
<div class="panel" :class="alpineClasses" style="margin: 5px;">Alpine</div>
134+
135+
<!-- Multiple inline styles -->
136+
<div style="display: flex;">Regular</div>
137+
</body>
138+
</html>';
139+
140+
$middleware = new InlineCss();
141+
$result = $middleware->apply($html);
142+
143+
// Framework-specific class attributes must be preserved
144+
$this->assertStringContainsString('ng-class="{expanded: isExpanded}"', $result);
145+
$this->assertStringContainsString('v-bind:class="vueClasses"', $result);
146+
$this->assertStringContainsString(':class="alpineClasses"', $result);
147+
148+
// Inline styles should be converted to page_speed classes
149+
$this->assertStringNotContainsString('style="border: 1px solid;"', $result);
150+
$this->assertStringNotContainsString('style="padding: 10px;"', $result);
151+
$this->assertStringNotContainsString('style="margin: 5px;"', $result);
152+
$this->assertStringNotContainsString('style="display: flex;"', $result);
153+
154+
// Should have style tag with converted classes
155+
$this->assertStringContainsString('<style>', $result);
156+
}
157+
158+
/**
159+
* Test Issue #133: Ensure the fix doesn't break normal class handling
160+
*/
161+
public function test_normal_class_still_works(): void
162+
{
163+
$html = '<!DOCTYPE html>
164+
<html>
165+
<head></head>
166+
<body>
167+
<div style="width: 100%;">
168+
<p style="font-weight: bold;">Bold text</p>
169+
<span style="color: yellow;">Highlighted</span>
170+
</div>
171+
</body>
172+
</html>';
173+
174+
$middleware = new InlineCss();
175+
$result = $middleware->apply($html);
176+
177+
// Inline styles should be converted to page_speed classes
178+
$this->assertStringContainsString('<style>', $result);
179+
$this->assertStringNotContainsString('style="width: 100%;"', $result);
180+
$this->assertStringNotContainsString('style="font-weight: bold;"', $result);
181+
$this->assertStringNotContainsString('style="color: yellow;"', $result);
182+
183+
// Should have page_speed classes instead
184+
$this->assertStringContainsString('class="page_speed_', $result);
185+
}
186+
187+
/**
188+
* Test: Edge case - attribute that ends with "class"
189+
*/
190+
public function test_attribute_ending_with_class(): void
191+
{
192+
$html = '<!DOCTYPE html>
193+
<html>
194+
<head></head>
195+
<body>
196+
<div data-css-class="some-value" style="background: blue;">
197+
Content
198+
</div>
199+
</body>
200+
</html>';
201+
202+
$middleware = new InlineCss();
203+
$result = $middleware->apply($html);
204+
205+
// Attribute ending with "class" should not be affected
206+
$this->assertStringContainsString('data-css-class="some-value"', $result);
207+
208+
// Inline style should be converted
209+
$this->assertStringNotContainsString('style="background: blue;"', $result);
210+
}
211+
212+
/**
213+
* Test: Complex real-world scenario with multiple frameworks
214+
*/
215+
public function test_complex_multi_framework_scenario(): void
216+
{
217+
$html = '<!DOCTYPE html>
218+
<html ng-app="app">
219+
<head></head>
220+
<body>
221+
<div id="vue-app">
222+
<!-- Navigation with Alpine.js -->
223+
<nav style="background: white;" x-data="{ open: false }">
224+
<button style="padding: 10px;" :class="{ active: open }">Menu</button>
225+
<ul style="list-style: none;" :class="open ? \'show\' : \'hide\'">
226+
<li style="margin: 5px;">Item</li>
227+
</ul>
228+
</nav>
229+
230+
<!-- Content with Vue.js -->
231+
<main style="min-height: 100vh;" v-bind:class="contentClasses">
232+
<div style="border: 1px solid;" ng-class="{loading: isLoading}">
233+
<h1 style="font-size: 24px;">Title</h1>
234+
<p style="line-height: 1.5;">Description</p>
235+
</div>
236+
</main>
237+
</div>
238+
</body>
239+
</html>';
240+
241+
$middleware = new InlineCss();
242+
$result = $middleware->apply($html);
243+
244+
// All framework-specific attributes must be preserved
245+
$this->assertStringContainsString(':class="{ active: open }"', $result);
246+
$this->assertStringContainsString(':class="open ? \'show\' : \'hide\'"', $result);
247+
$this->assertStringContainsString('v-bind:class="contentClasses"', $result);
248+
$this->assertStringContainsString('ng-class="{loading: isLoading}"', $result);
249+
250+
// Inline styles should be converted
251+
$this->assertStringNotContainsString('style="background: white;"', $result);
252+
$this->assertStringNotContainsString('style="padding: 10px;"', $result);
253+
$this->assertStringNotContainsString('style="border: 1px solid;"', $result);
254+
$this->assertStringNotContainsString('style="font-size: 24px;"', $result);
255+
256+
// Should have style tag
257+
$this->assertStringContainsString('<style>', $result);
258+
}
259+
}

0 commit comments

Comments
 (0)