988702 - fix missing UngetToken() calls in ParseCounterData

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I've had a patch in my tree for ages to fix missing UngetToken() calls in ParseCounterData() (which leads to incorrect parsing if the token we forgot to unget is relevant to matching (), [], or {}); it was just waiting for me to write tests, which I just did.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1 - Remove duplication between invalidGradientAndElementValues and the list of invalid values in background-image

Values that are already in invalidGradientAndElementValues don't also
need to be listed in an array that has invalidGradientAndElementValues
appended to it.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2 - Test invalid CSS property values for their balanced-ness of () [] or {}

(This provides an easy mechanism for testing for missing UngetToken()
calls as in the following patch.)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 3 - Fix missing UngetToken() calls in ParseCounterData(), and avoid evil CheckEndProperty() calls while I'm there

I tested that test_property_syntax_errors.html gives a failure on the
new test without the patch, and that the failure is not present with the
patch.

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8397566 [details] [diff] [review]
patch 1 - Remove duplication between invalidGradientAndElementValues and the list of invalid values in background-image

Review of attachment 8397566 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/property_database.js
@@ +1924,5 @@
>  		"none, url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), none",
>  		"url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==)",
>  		].concat(validGradientAndElementValues),
>  		invalid_values: [
> +			

Remove the blank line.

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8397567 [details] [diff] [review]
patch 2 - Test invalid CSS property values for their balanced-ness of () [] or {}

Review of attachment 8397567 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/test/property_database.js
@@ +766,5 @@
>  		invalid_values: [
>  			"url('border.png') url('border.png')",
> +		].concat(invalidGradientAndElementValues),
> +		unbalanced_values: [
> +			

Remove the blank line.

@@ +1937,5 @@
>  		invalid_values: [
>  			
> +		].concat(invalidGradientAndElementValues),
> +		unbalanced_values: [
> +			

And this one too.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5015f2ffabfd
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe9e2cdcfdc7
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9efc607db23

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5015f2ffabfd
https://hg.mozilla.org/mozilla-central/rev/fe9e2cdcfdc7
https://hg.mozilla.org/mozilla-central/rev/c9efc607db23