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

Fix #3466: Crash happened when make some operation with decimal number in the Tab Size/Spaces field. #3487

Merged
merged 1 commit into from Apr 24, 2013

Conversation

TomMalbran
Copy link
Contributor

Fix for issue #3466 by not letting the unit be a float value, by making it an integer before saving it.

@ghost ghost assigned RaymondLim Apr 19, 2013
@peterflynn
Copy link
Member

Even though this guards us against it, we should probably file a bug on CodeMirror too just to be a good citizen -- probably no client of CM would be happy with it going into an infinite loop when given a malformed tabs/spaces setting.

@TomMalbran
Copy link
Contributor Author

Right, but the manual specifies that the indentUnit and the tabSize should be integers, so it seems more of our fault for providing a float to the option. I checked the CodeMirror code an neither of the options have a guard against bad values supplied to them. But if this seems that should be fixed in CodeMirror too, then we should add a bug there too.

@njx
Copy link
Member

njx commented Apr 22, 2013

Yup, I think we want to take this fix. @RaymondLim - please merge this sprint. Thanks!

@RaymondLim
Copy link
Contributor

I can't reproduce the infinite loop with a CodeMirror demo page. The only thing I can reproduce is the word "undefined" gets inserted at the beginning of the new line as indentation. Anyway, I will try to log one for this later.

For now this Looks good and we can merge.

RaymondLim added a commit that referenced this pull request Apr 24, 2013
Fix #3466: Crash happened when make some operation with decimal number in the Tab Size/Spaces field.
@RaymondLim RaymondLim merged commit 26e9c90 into adobe:master Apr 24, 2013
@TomMalbran TomMalbran deleted the tom/fix-issue-3466 branch April 24, 2013 06:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants