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

Fix a second infinite loop in CSSUtils.findSelectorAtDocumentPos() #9295

Merged
merged 2 commits into from
Sep 25, 2014

Conversation

peterflynn
Copy link
Member

Fix a second infinite loop in CSSUtils.findSelectorAtDocumentPos(), caused by cleanups in the fix for the earlier infinite loop bug (#9002). To keep loop exit condition more precisely in sync with TokenUtils's logic for when to stop advancing, create new TokenUtils methods for checking if at a stopping point.

The existing unit test "should find the first selector when pos is at beginning of selector name" already hits the case that was broken here -- basically a curly brace that's not alone on a line, afaict -- so I didn't bother adding any additional unit tests in this PR.

@RaymondLim @marcelgerber

…aused

by cleanups in the fix for the earlier infinite loop bug. To keep loop
exit condition more precisely in sync with TokenUtils's logic for when to
stop advancing, create new TokenUtils methods for checking if at a stopping
point.
@RaymondLim
Copy link
Contributor

Reviewing ...

@@ -76,6 +76,11 @@ define(function (require, exports, module) {
return true;
}

/** Returns true if movePrevToken() would return false without changing pos */
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Can you add @param and @return?

@RaymondLim
Copy link
Contributor

Looks good! Just have minor nits for JSDoc. Ran all tests and it does fix the CSSUtils unit test failure.

@peterflynn
Copy link
Member Author

@RaymondLim Changes pushed. I fixed a bunch of other type annotations in that file at the same time, since it turned out many of the existing ones were inaccurate or had bad syntax.

@RaymondLim
Copy link
Contributor

Looks good! Merging.

RaymondLim added a commit that referenced this pull request Sep 25, 2014
Fix a second infinite loop in CSSUtils.findSelectorAtDocumentPos()
@RaymondLim RaymondLim merged commit f5dd2e7 into master Sep 25, 2014
@RaymondLim RaymondLim deleted the pflynn/cssutil-infinite-loop2 branch September 25, 2014 22:47
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.

2 participants