[GHS] Disallowed attribute is preserved after calling setData() twice if RegExp contains global search flag

[Mgsy]

📝 Provide detailed reproduction steps (if any)

• Add the following configuration to the GHS feature:

allow: [
	{
		name: /.*/,
		styles: true,
		classes: true,
		attributes: true
	}
],
disallow: [
		{
			attributes: [ { key: 'srcdoc', value: /^<strong>/g } ]
		}
]

• Call editor.setData( '<iframe srcdoc="<strong>bar</strong>"></iframe>' ); x2.

✔️ Expected result

The <iframe> appears without srcdoc attribute.

❌ Actual result

editor.setData( '<iframe srcdoc="<strong>bar</strong>"></iframe>' );
editor.getData();
"<p><iframe></iframe></p>"

editor.setData( '<iframe srcdoc="<strong>bar</strong>"></iframe>' );
editor.getData();
"<p><iframe srcdoc=\"<strong>bar</strong>\"></iframe></p>"

📃 Other details

The issue occurs only if there's a g flag in the RegExp. 

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[maxbarnas]

I suspect that the flag y can also trigger this bug.

[maxbarnas]

The problem seems to come from the fact we have a stateful regular expression object, which is not reset after every match. One approach is to reset it manually, using lastIndex property. Another solution might be to not call test() on regular expression but call match() on the tested string while providing the regexp object as a param.

I tried both solutions and for the safety benchmarked setData() method to verify we are not introducing any performance problems. Each version was tested 5 times, each time on a new tab:

Small test - 16K characters of an input

RegExp.test() + lastIndex = 0 [ms]	String.prototype.match() [ms]
151	144
153	152
152	148
152	153
154	141

Large test - 160K characters of an input

RegExp.test() + lastIndex = 0 [ms]	String.prototype.match() [ms]
1943	2012
1540	1585
1528	1534
1545	1593
1543	1543

As we can see, the difference seems to be negligible.

[maxbarnas]

@Mgsy @niegowski @Reinmar

While testing the fix, I found that the current behavior of StyleProcessor might not be correct. This makes few tests fail, as the new code corrects the bug, where previously it was ignored and returning a false positive match.

With Matcher trying to match border-left: 1px solid to border-left: /*./ but having only normalized styles like:

• border-left-width: 1px,
• border-left-style: solid.

the match never succeeds.

Reducer assumes that we need all 3 values of the border to return a shorthand, e.g. border-left: 1px solid blue. While looking at the specification I confirmed that two or one value is also a valid CSS expression: https://drafts.csswg.org/css-backgrounds-3/#border-shorthands .

Do we want to fix that now or should I just modify the tests for the time being and create a follow-up?

[maxbarnas]

In the end, I marked the problematic test as skipped, as it brings no value with the current, incorrect behavior of style reducers.