Community
Participate
Working Groups
Created attachment 205982 [details] proposed patch with unit test Some intended cleverness for appending semicolons to the translated content from event handler attributes, e.g. onload="", causes a SIOOBE if the value is empty. This can happen quickly if the attribute was inserted by content assist or the user just hasn't filled it in, yet. It prevents most JS support in web pages from working properly. java.lang.StringIndexOutOfBoundsException: String index out of range: -1 at java.lang.String.charAt(String.java:686) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslator.translateInlineJSNode(JsTranslator.java:402) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslator.translate(JsTranslator.java:326) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslator.reset(JsTranslator.java:275) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslator.<init>(JsTranslator.java:171) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslation.getTranslator(JsTranslation.java:100) at org.eclipse.wst.jsdt.web.core.javascript.JsTranslation.getJsText(JsTranslation.java:378) ...
*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. The exception derails the translation on perfectly harmless files, preventing most JavaScript support in web pages from working. *Is there a work-around? If so, why do you believe the work-around is insufficient? The user has to fill in a value. It's not obvious, though, as we don't report this necessity through validation or other means. *How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? Ad-hoc and with attached unit test. *Give a brief technical overview. Who has reviewed this fix? The translator was checking if the last character of the event handler value was a ';', after stripping off any surrounding quotes. It did not properly check to see whether there were _any_ characters in the value (non-zero length), first. We're proposing to roll back that one line. Reviewed by Chris. *What is the risk associated with this fix? Very low. The semicolon goes back to always being inserted.
This sounds like an important fix, and I could not object to the fix proposed in the patch, but using "charAt" is generally not recommended by "NL enablement" experts, as its doesn't support some of the more unusual languages (16 or 32 bit?) ... I'm not sure we support those esoteric languages in general, but, if there was an ICU equivalent (codePointAt?) it would be a slightly better fix.
(In reply to comment #2) > This sounds like an important fix, and I could not object to the fix proposed > in the patch, but using "charAt" is generally not recommended by "NL > enablement" experts, as its doesn't support some of the more unusual languages > (16 or 32 bit?) ... I'm not sure we support those esoteric languages in > general, but, if there was an ICU equivalent (codePointAt?) it would be a > slightly better fix. It was used handling JavaScript source code, not natural language, and is removed by the patch anyway.
Oh, thanks for clarifying that I was readying the patch "backwards".
Seems like a bad problem with safe fix.. approve
+1.
Released.
