Opened 5 years ago

Closed 5 years ago

#11085 closed Bug (fixed)

IE8 and MathJax

Reported by: Piotr Jasiun Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3.1
Component: General Version:
Keywords: Cc:

Description

There is a problem with MathJax in IE8.

I think that we should replace MathJax with placeholder there.

Change History (11)

comment:1 Changed 5 years ago by Piotr Jasiun

Status: newconfirmed

comment:2 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:3 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

IE8 use different frame wrapper which shows only TeX instead of MathJax. Changes in t/11085 and corresponding test branch.

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

Status: reviewreview_failed
  1. JSLint issues.
  2. JSDuck issues.
  3. JSDuck comments do not conform the style guide for documentation.
  4. I think that it would be nice if MathJax preview (both in dialog and in editable) could be disabled also on other than IE8 browsers. For example on those on which MathJax works very slowly. So this could be configurable.
Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:5 in reply to:  4 Changed 5 years ago by Piotr Jasiun

Status: review_failedreview

Replying to Reinmar:

  1. JSLint issues.
  2. JSDuck issues.
  3. JSDuck comments do not conform the style guide for documentation.

I fixed JSLint and JSDuck issues.

  1. I think that it would be nice if MathJax preview (both in dialog and in editable) could be disabled also on other than IE8 browsers. For example on those on which MathJax works very slowly. So this could be configurable.

IE8 placeholder, which is displayed instead of preview, is created only for IE8 and tested only with it. If we want to give users option to use placeholder or preview in any browser we should create more tests and spend some time on this and now we have other important issues to work on.

comment:6 Changed 5 years ago by Olek Nowodziński

Status: reviewreview_failed

Instead of

if( CKEDITOR.env.ie && CKEDITOR.env.version == 8 )
    CKTESTER.test( {} );

and wrapping all tests in conditional statement, the following seems much cleaner:

'async:init' : function() {
    if ( CKEDITOR.env.ie && CKEDITOR.env.version == 8 )
        assert.ignore();

    ...
}

Also the following in regressions.js wouldn't work anyway (notation requires .html#test_name):

'/dt/plugins/mathjax/mathjax.html': 'env.ie && env.version == 8',

Anyway, what piece of logic controls the width of the placeholder?

comment:7 Changed 5 years ago by Piotrek Koszuliński

Owner: changed from Piotr Jasiun to Piotrek Koszuliński
Status: review_failedassigned

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

Status: assignedreview

I fixed the broken tests, which were ignored from 2 places at the same time.

Question which remains is:

Anyway, what piece of logic controls the width of the placeholder?

comment:9 in reply to:  8 Changed 5 years ago by Piotr Jasiun

Question which remains is:

Anyway, what piece of logic controls the width of the placeholder?

This: https://github.com/cksource/ckeditor-dev/blob/t/11085/plugins/mathjax/plugin.js#L414

comment:10 Changed 5 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:11 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:4cdcded on dev and e48344b on tests.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy