Opened 10 years ago

Closed 10 years ago

#12295 closed Bug (fixed)

Red test on master: mathjax-mock - test conflict with iframe plugin

Reported by: Piotr Jasiun Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version: 4.4.2
Keywords: Cc:

Description (last modified by Piotr Jasiun)

Test is red on Chrome 36.0.1985.125 and Firefox 31.0, green on IE11.

Test is red because of changes in bender-ckeditor (benderjs-ckeditor t/12173).

Change History (13)

comment:1 Changed 10 years ago by Piotr Jasiun

Description: modified (diff)
Milestone: CKEditor 4.4.4CKEditor 4.5.0
Owner: set to Piotr Jasiun
Status: newassigned

comment:2 Changed 10 years ago by Piotrek Koszuliński

Make sure it does not affect master (4.4.x). These changes should not land on benderjs-ckeditor's master yet or must be immediately fixed on ckeditor-dev/master.

comment:3 Changed 10 years ago by Piotr Jasiun

Maybe we should have both master and major on bender-ckeditor to handle such cases like this?

comment:4 Changed 10 years ago by Piotrek Koszuliński

I'm afraid that it's not a good idea. You'll need again switch between branches in two repositories when switching between them in ckeditor-dev. We need to ask Grzesiek if it's possible to merge benderjs-ckeditor into ckeditor-dev. We wanted to do that some time ago, but it wasn't possible to load Bender plugins from local directories.

comment:5 Changed 10 years ago by g.pabian

Using local paths for Bender's plugins is possible for a long time (since Bender 0.0.12). You can move this plugin to ckeditor-dev freely. Just let me know when it's done so I can remove the unused repository.

comment:6 Changed 10 years ago by Piotrek Koszuliński

@pjasiun: Will you handle that? We should do this change on master, so we'll be able to close this issue on master and major at the same time.

BTW. Where we can put benderjs-ckeditor? I see at least 3 options:

  • in new directory /benderjsplugins/ (don't like creating new directory in ckeditor-dev root)
  • somewhere inside /dev/ (will be a problem when building for tests)
  • in /tests/_assets/ (odd) or /tests/benderplugins/ (odd inception, but I like it the most)

comment:7 in reply to:  6 Changed 10 years ago by Piotr Jasiun

Replying to Reinmar:

@pjasiun: Will you handle that? We should do this change on master, so we'll be able to close this issue on master and major at the same time.

BTW. Where we can put benderjs-ckeditor? I see at least 3 options:

  • in new directory /benderjsplugins/ (don't like creating new directory in ckeditor-dev root)
  • somewhere inside /dev/ (will be a problem when building for tests)
  • in /tests/_assets/ (odd) or /tests/benderplugins/ (odd inception, but I like it the most)

Sure, I will handle this. I also think that we should put it into /tests/ directory. It is not odd. Tests directory means "everything related to tests". We have there assets and helpers, so tests environment plugins should goes there too.

I am not sure if we should move all plugins or only the bender-ckeditor plugin. If we want to move all, then we should definitely use sub repositories.

In my opinion we should just move bender-ckeditor to /tests/_benderjs-tools/ and remove bender-ckeditor repository.

comment:8 Changed 10 years ago by Piotrek Koszuliński

So if we put it in /tests/ the name should be benderjsplugins or _benderjsplugins, because these are plugins, not tools. We are now merging only one plugin into ckeditor-dev, but in the future we may have more Bender plugin specific for CKEditor, so that's why I want to have a grouping directory.

comment:9 Changed 10 years ago by Piotr Jasiun

bender-ckeditor was merged into CKEditor repository in #12296 ticket.

comment:10 Changed 10 years ago by Piotr Jasiun

Red test fixed. Changes in t/12295.

comment:11 Changed 10 years ago by Piotr Jasiun

Status: assignedreview

comment:12 Changed 10 years ago by Piotr Jasiun

Rebased on major.

comment:13 Changed 10 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Fixed on major with git:409e8d5.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy