Community
Participate
Working Groups
In resolving Bug 326193, it seems like certain scenarios started springing up that would cause unclosed comment errors to be generated. The most recent one would occur for something like: <!--<link HREF='<c:out value='localhost/file.css'/>'>--> To correct this, we'll be backing out the changes made and employing a new strategy. Instead, we'll go back to relying on the XMLJSPRegionHelper to properly decode these commented regions.
Created attachment 188723 [details] patch Tokenizer: changes reverted (probably a performance improvement too since we're not creating as many text regions (almost one was being created per character in comments) and the buffer was copied into a string each time a comment was scanned XMLJSPRegionHelper: The contents of a comment are translated as a whole instead of by each region within. This is because the tokenizer would create a new text region separate from the start tag when it was included in a comment
Created attachment 188934 [details] unit tests Added unit tests for the scenario outlined in the description to make sure that the comment is properly closed. Additionally, a unit test has been added to cover another scenario that is paired with this "comment unclosed" issue, which was that if any markup was included in an HTML comment (e.g., <!-- <div></div> -->) each character was becoming its own text region, increasing memory usage and parsing time. This unit test covers to make sure that the comment remains a contiguous XML_COMMENT_TEXT.
Created attachment 188943 [details] unit tests (one more)
Created attachment 188961 [details] handling of comment with a tag
Created attachment 188962 [details] handling of comment with an embedded tag
Created attachment 188963 [details] error message shown for correct text
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. It's a functional and performance regression from the prior release. * Is there a work-around? If so, why do you believe the work-around is insufficient? There is no workaround. It's a defect in our low-level tokenizer and an associated class. * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? Manually tested by Nick, also with the included unit tests written by Nick and myself. * Give a brief technical overview. Who has reviewed this fix? Changes how the parser handles tags within XML comments so that they don't affect that part of the state machine; a comment end will be recognized regardless of what precedes it, as before. * What is the risk associated with this fix? The affected area is wide-reaching, but we've lowered the risk as much as we can. Low.
This is definitely a stop-ship and must be fixed.
Verified with 3.2.3-20110216062908 candidate, both visually in the Outline and with our info dialog.
assuming status should be "fixed"?
Yes
