Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initialize plugins before the formatters #822

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

live627
Copy link
Collaborator

@live627 live627 commented Apr 14, 2021

Plugins should be initialized before the formatters since they may wish to add or change formatting handlers and since the bbcode format caches its handlers, such changes must be done first.

One possible caveat is that the public API sceditor.command won't work as intended in plugin init—#686 works around this by duplicating some code

	base.commands = utils
		.extend(true, {}, (userOptions.commands || defaultCommands));

That workaround seems kinda ugly to me, and will probably cause some undesired conflicts when running multiple editor instances on the same page. I haven't tested that, though.

Come to think of it, sceditor.formats.bbcode may also exhibit the same behavior due to what looks like shared memory access. I have not tested this either.

I see more of what could lead to surprising results: methods that get pointers to iframe related objects return null, range helper also is null, but those can easily be worked around by waiting for the ready signal.

Plugins should be initialized before the formatters since they may wish to add or change formatting handlers and since the bbcode format caches its handlers, such changes must be done first.
@github-actions github-actions bot added the lib label Apr 14, 2021
@live627 live627 added this to the v3.1.0 milestone Apr 16, 2021
@live627 live627 merged commit 6cdfc5a into samclarke:master Jun 25, 2021
@live627 live627 deleted the plugins-order branch June 25, 2021 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant