Bugzilla – Full Text Bug Listing
| Summary: | [nls tooling][quick assist] IAE with 'Remove key' quick assist in Properties File editor | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Raksha Vasisht <raksha.vasisht> |
| Component: | Text | Assignee: | Markus Keller <markus.kell.r> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P2 | CC: | daniel_megert, deepakazad, markus.kell.r |
| Version: | 3.8 | Flags: | daniel_megert:
review+
|
| Target Milestone: | 3.7.2 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Raksha Vasisht
Deepak, do you have time to fix this for M3 (i.e. today or tomorrow)? I quickly tried with HEAD of master and cannot reproduce the exception. Raksha, do you have a test case ? I would need messages.properties and Messages.java files. > -> The additional info is empty except for '...' (or wrong in most cases) I half expected bugs about exceptions in this code, as it relies on document partitioning and things can go blow up there. But I am a bit surprised by "wrong in most cases", according to me if the additional info shows up it should be correct in almost all cases. Again, can you please share your test cases? (In reply to comment #0) > 2) click on 'Remove key' proposal Does it make a difference if you select the quick assist via keyboard instead of mouse ? (I don't think it should, but asking just to be sure.) Dani, I don't think I would be able to do a complete fix by tomorrow. Ok to push this to M4? (In my opinion the quick assists fail only in rare scenarios) Note to self: Things can blow up if a key includes special characters e.g. .(dot), or you have something like "a a = x" in one line in the properties file (the space between 2 'a's is important here). But these are different exceptions from comment 0 and the additional info does not show up in these cases. (In reply to comment #3) > Dani, I don't think I would be able to do a complete fix by tomorrow. Ok to > push this to M4? (In my opinion the quick assists fail only in rare scenarios) Yes. I'll take a look for M3 in case Raksha provides a test case that indicates a more frequent occurrence of the IAE. I was not able to trigger the IAE during my test pass. To reproduce, remove the last key in /org.eclipse.core.expressions/src/org/eclipse/core/internal/expressions/ExpressionMessages.properties, and then remove the next last key. Only seems to happen with Windows line delimiters. The code is a total disaster. Problems occur whenever there is whitespace around key and/or value, see the trimming in PropertyFileDocumentModel#parsePropertyDocument(IDocument) and PropertyFileDocumentModel.KeyValuePairModell#getLength() that uses the trimmed values. Dani, I think we should backport this to 3.7.2. I guess it only got broken in 3.7 due to bug 333631 (didn't verify this). Fixed in master. commit b4c16ccdb9d004714b17b3f3873ca6d1b00396e6 (In reply to comment #6) > To reproduce, remove the last key in > /org.eclipse.core.expressions/src/org/eclipse/core/internal/expressions/ExpressionMessages.properties, > and then remove the next last key. Interesting. I can still not reproduce the IAE with those steps. I took the file from I20111025-1800 and tried with Windows and Unix LDs. What I can reproduce though is the scrambling of the file. (In reply to comment #8) > The code is a total disaster. I agree. > Dani, I think we should backport this to 3.7.2. I guess it only got broken in > 3.7 due to bug 333631 (didn't verify this). Yes, I just verified this and +1 to backport. Thanks Markus for looking into it. Verified in I20111026-2132 that the file is no longer scrambled when removing a property. Still unable to reproduce the IAE. > Interesting. I can still not reproduce the IAE with those steps. I took the
> file from I20111025-1800 and tried with Windows and Unix LDs.
My bad: I was using the latest build and this already had Markus's fix in it. So that also proofs that the fix works fine.
(In reply to comment #2) > I quickly tried with HEAD of master and cannot reproduce the exception. Raksha, > do you have a test case ? I would need messages.properties and Messages.java > files. > I just tried ActionMessages.java and ActionMessages.properties and also CorrectionMessages.java and CorrectionMessages.properties. > > -> The additional info is empty except for '...' (or wrong in most cases) > I half expected bugs about exceptions in this code, as it relies on document > partitioning and things can go blow up there. But I am a bit surprised by > "wrong in most cases", according to me if the additional info shows up it > should be correct in almost all cases. The info either shows only '...' , or some weird characters like '<br>' , or sometimes only the previous/next entry in the properties file and blank afterwards. Same test case as above. > > (In reply to comment #0) > > 2) click on 'Remove key' proposal > Does it make a difference if you select the quick assist via keyboard instead > of mouse ? (I don't think it should, but asking just to be sure.) No. (In reply to comment #9) > Interesting. I can still not reproduce the IAE with those steps. I took the > file from I20111025-1800 and tried with Windows and Unix LDs. What I can > reproduce though is the scrambling of the file. You need to set windows delimiters and also have a space after '='. Also, the file becomes ineditable afterwards, and you need to close it and reopen to edit it again. Verified that the fix works fine with I20111026-2132 and can longer see the IAE or additional info issue. Fixed in R3_7_maintenance: 4f0105ccd524752d55e59309efd792ac0037f7db Verified in M20111128-1153. |