Fix ACE module files failing to load #82
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the ACE editor is initialized, by default the base path used to load module files is set to the directory of the "ace.js" file. Since the JavaScript files were merged "ace.js" is no longer added to the page as a single script file, so the base path was no longer set, and thus the module files failed to load. Now the right base path is explicitly set in the editor configuration. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
rullzer
approved these changes
Feb 7, 2018
This was referenced Feb 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes (in master) #39, which is a regression introduced in #32
When an ACE module is loaded, if the module is not already defined, the file that should contain the definition of the module is loaded. If no explicit URL was set for the module file, the URL of the file to load is based on the module name and the path configured for the component type (
worker,modeortheme); if no explicit path was set or the module has no special type then thebasePathconfiguration option is used instead.When the ACE editor is initialized, by default the base path is set to the directory of the ace.js file. When the JavaScript files were merged ace.js was no longer added to the page as a single script file, so the base path was no longer set, and thus the module URL was just based on its name, without a path. Due to this the URL for the ace/ext/searchbox module ended being just ext-searchbox.js, and as such it was relative to the URL of the current page (and thus, wrong). Note, however, that themes and syntax highlighting worked fine because the needed files were explicitly added to the page.
In order to fix this the right base path had to be set in the editor configuration.
Besides master, this fix is needed too in the stable12 and stable13 branches. It brings back the search box, and also the syntax check in stable12; in stable13 and master the syntax check is not working either since the update to ACE 1.2.8.; something for another pull request ;-)
@nextcloud/javascript