This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Discovered while working on defect # : keymaps store the shortcuts as DataShadows pointing to actions triggered by the keystroke. However if the action .instance file moves/renames (e.g. WhereUsed during 7.1 release), the shortcuts become broken. Specifically broken DataShadows that point on SFS (layer) is a more general case than just shortcuts, so I think some translation mechanism could be used to translate moved paths. Current Utilities.translate() is capable of such function, except its Javadoc claims it can be only used to translate classnames. Attaching a patch to DataShadow + javadoc clarification in Utilities for review.
Created attachment 116046 [details] Proposed changes
(In reply to comment #0) > Discovered while working on defect # : What issue? Should it not be dependent on this? [JG01] Javadoc omits ".instance" suffix in value side; accident? Actions/Refactoring/RefactoringWhereUsed.instance=Actions/Refactoring/org-netbeans-modules-refactoring-api-ui-WhereUsedAction [JG02] urlText.substring(prefix.length()) and new URI(prefix + translated) are not in general correct ways to interconvert between URI fragments and FileObject paths; try it with "Actions/Some Category/My Action #1.instance" and you will see why. URI.relativize is not appropriate either, since the return value is still an encoded URI, and similarly for URI.resolve. In general you need to use the URI.path field to properly escape and unescape path components. (Note that for FOs on the SFS, URI.path != FO.path, since there is normally a "nbhost/" prefix, so you still need to compare to SFS root.) [JG03] translated != cfgPart assumes that that U.t returns the identical String object if not translated. Safer to use !equals. [JG04] Your code does not seem to handle the second branch of the if-clause, where path is not a URI but an actual FileObject path (meaning JG02 does not apply here). [JG05] No unit test, especially for JG02 and JG04.
Created attachment 116136 [details] Patch addressing JG01-JG05 [JG01] corrected. [JG03] Utilities.translate is documented to return the exactly same reference if no translation is defined. But changing to be more robust. [JG02] Correct, thanks. I still think, that comparing encoded URIs for substring to find whether URI belongs to SFS is valid - both should be translated the same way. The composition was NOT correct, as you demonstrated - so I changed that part [JG04] Case added [JG05] Testcases added
[JG06] Use toString, not toASCIIString. You do not want non-ASCII chars translated to some weird escapes here - just leave them alone. [JG07] cfgRootURI.resolve(translated) is probably wrong as I said in JG02 - this will throw IAE if translated contains spaces or the like. Remember that translate.names keys and values are file paths, not URI fragments, contrary to what your unit test seems to be expecting. (Why Utilities does not just use java.util.Properties, I am not sure, but anyway it reads the file in UTF-8 and splits on the first '=' or ' '.)
re.: contents of translate.names: actually the format depends on definition being put in javadoc. The syntax of the file is currently defined so that lines are cut after first '#' (comments). So at least # should be somehow escaped. Instead of inventing yet another escaping scheme, I would prefer to use standard URL encoding instead. re.: excessive escaping (toASCIIString) - will try to change to less obtrusive form; translate.names is read as UTF-8 after all. re.: resolve(translated) throwing exception - will not throw, if contents of translate.names is according to rules (e.g. all spaces must be %-encoded). Sadly the URL encoding is already consumed for # escaping, so either we need to accept URL fragments as key/value pairs, or invent special encoding for #.
(In reply to comment #5) > actually the format depends on definition being put in javadoc. If you are adding the ability to translate paths then I suppose you can invent a new format and document it. > the URL encoding is already consumed for # escaping Do not understand what this sentence means but perhaps it does not matter.
(In reply to comment #6) > (In reply to comment #5) > > actually the format depends on definition being put in javadoc. > > If you are adding the ability to translate paths then I suppose you can invent > a new format and document it. OK, so I can as well change the javadoc to specify that the filenames should be written as URI paths with the appropriate encoding. For completness, we also may need to escape '=' as well (another char illegal for class names, but permited for filenames). All is about the decision what rules are easier to follow and code complexity 1/ filenames, but encode # and = using some (RFC 2396?) encoding. In this case, we must change handling of ' ' in code, but most of filenames will be written in a natural way. 2/ specify filenames as URI paths PLUS encode '=' using RFC 2396, if part of filename. This scheme matches what FileObject.toURI() produces for a FileObject, except '='. 3/ other encoding system, which translates # and = to other characters What do you suggest, from the developer's perspective ?
Or 4. Document that '=' and any characters (including ' ') which would need URI escaping may not be used in either the key or value in translate.names, unless and until someone needs such a feature. Normally layer files are given fairly conservative filenames - ^[/a-zA-Z0-9$_.+-]+$ - with only a few exceptions: Actions/Build/Set Request Parameters.instance, Actions/Help/Show BluePrints.instance, Actions/Help/Show Welcome.instance - bad practice J2EE/DeploymentPlugins/APPSERVERSJS/DeploymentFileNames/CAR/META-INF\sun-application-client.xml and similar - ???? Keymaps/Emacs/C-X 1.shadow and similar - OK, required by multikey syntax Templates/Project/Web/Html Project - bad practice Toolbars/Jemmy Support/ - bad practice so it is unlikely you would need to translate them, I think.
OK, given you're the original author of the counter-example (with chars like '#', ' '), I would gladly agree with (4), as the simplest, yet solving the most common cases. Filenames on sysfs (cfg) could be 'sane', if we want to write them on FAT32 fs ... if you agree, I'll simplify the patch back to non-URI and change the javadoc appropriately.
The motivation for JG02 was that your code was pretending to work with all filenames but really would not. If it is easier to just explicitly reject certain pathological cases then do so and document it.
Created attachment 116763 [details] Disallowed special chars in translated filenames [JG02] Addressed by documentation for Utilities.translate [JG04], [JG05] addressed by previous patch; removed unnecessary testcases. [JG06], [JG07] become IMHO obsolete because of constraints on allowed filenames.
OK from me.
Changed in jet-main #4c80493a0c8c
Integrated into 'main-golden', will be available in build *201203240400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/4c80493a0c8c User: Svata Dedic <sdedic@netbeans.org> Log: #208779: DataShadows use translate.names if the original file could not be found - bw compatibility
Integrated into 'main-golden', will be available in build *201203250401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/4c80493a0c8c User: Svata Dedic <sdedic@netbeans.org> Log: #208779: DataShadows use translate.names if the original file could not be found - bw compatibility