Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix minor TimingFunction validation issue #8910

Merged

Conversation

marcelgerber
Copy link
Contributor

This fixes a rather uncommon issue in TimingFunction validation. Steps:

  1. Try to open an Inline Editor on animation-timing-function: cubic-bezier(0, 0, 0, 0,) (notice the trailing comma)

Result: Console error, no inline editor
Expected: Inline editor should auto-correct the invalid function

@redmunds redmunds self-assigned this Sep 2, 2014
}

var val = parseFloat(str, 10),
isNum = (typeof (val) === "number") && !isNaN(val) &&
isNum = (typeof val === "number") && !isNaN(val) &&
(val !== Infinity) && (val !== -Infinity);
Copy link
Contributor

Choose a reason for hiding this comment

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

@marcelgerber This is an interesting case. Due to the regex used to parse the cubic-bezier string, the last value is "everything past the 3rd comma and past whitespace". So there could be a lot of weird stuff in here -- too much I think to try to parse with our own regex.

So, I looked at the code that currently traps the exception and it's using the unary operator to convert from string to number. I think we still need parseFloat to make sure it's a base 10 number, but this code seems to be simpler, solve the problem, and all unit tests still work:

        if (typeof str !== "string") {
            return { isNumber: false, value: null };
        }

        var val = parseFloat(+str, 10),
            isNum = (typeof val === "number") && !isNaN(val) &&
                    (val !== Infinity) && (val !== -Infinity);

What do you think of this solution?

@redmunds
Copy link
Contributor

redmunds commented Sep 8, 2014

Code review complete. I think this can be solved without a new regex.

@marcelgerber
Copy link
Contributor Author

@redmunds Your solution works well, therefore I used it.

@@ -215,6 +215,9 @@ define(function (require, exports, module) {
it("should correct cubic-bezier function with 5 parameters", function () {
testInvalidBezier("cubic-bezier(0, 0, 1, 1, 1)", ["cubic-bezier(0, 0, 1, 1)", "0", "0", "1", "1"]);
});
it("should correct cubic-bezier function with trailing comma", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay! Unit test!

@redmunds
Copy link
Contributor

redmunds commented Sep 9, 2014

Good catch. Merging.

redmunds added a commit that referenced this pull request Sep 9, 2014
…eption

Fix minor TimingFunction validation issue
@redmunds redmunds merged commit 49e2004 into adobe:master Sep 9, 2014
@marcelgerber marcelgerber deleted the timing-function-editor-exception branch September 10, 2014 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants