Ticket #6126 (closed Bug: fixed)

Opened 4 years ago

Last modified 3 years ago

input with name="submit" and hefty css/js in IE7/8 causes editor to intermittently fail to load

Reported by: narchos Owned by: alfonsoml
Priority: Normal Milestone: CKEditor 3.5.1
Component: General Version: 3.3.2
Keywords: IE Cc:

Description

This is the same as Ticket #4726. I experienced exactly the same problem in IE7/8 with ckeditor 3.3.2. A page with 4 editors would intermittently load between 0 and 3 editors instead of four. Only error encountered was when using the IE8 debugger. It would complain exactly as in http://dev.ckeditor.com/ticket/4726.

I was able to increase the success rate by:

  1. setting a timeout on the CKEDITOR.replace() call, the longer the timeout the better the success at loading editors
  2. removing large css and js links from the HTML.

I was able to get the editors to load 100% reliably once I changed an input with name="submit" to name="somethingelse". 100% no worries at all, worked everytime.

I am guessing that the reason you guys might have had trouble replicating this is due to the hefty css/js that was contributing to the problem. With a minimal test page you would not be able to get this behaviour to occur.

Hope this assists you in making this awesome editor even more awesome!

If you would like a better test case PM me and I'll send you something

Attachments

ckeditor-test-script-6126.php (2.3 KB) - added by narchos 4 years ago.
test script to replicate issue 6126
6126.patch (1.5 KB) - added by alfonsoml 3 years ago.
Proposed patch
6126_2.patch (506 bytes) - added by alfonsoml 3 years ago.
Revised patch

Change History

comment:1 Changed 4 years ago by Saare

  • Status changed from new to pending
  • Keywords ie7 ie8 failed to load name="submit" removed

I'm not able to reproduce the issue. A test case would be great.

Changed 4 years ago by narchos

test script to replicate issue 6126

comment:2 Changed 4 years ago by narchos

I've attached a test case. Tested on IE8 (8.0.6001.18702 - Win XP SP3). You'll just need to update the link to the ckeditor-3.3.2 js file.

Looks like it actually requires TWO inputs with a name="submit" to cause problems. Sorry I didn't notice this page had a submit at the top and bottom when I raised this ticket.

Hope this helps to replicate the issue.

comment:3 Changed 4 years ago by tobiasz.cudnik

  • Keywords IE added
  • Status changed from pending to closed
  • Resolution set to wontfix

I'm able to reproduce this issue with two input[name=submit], but using more than one input with same name in one form in not valid. Your test page works well when we change second submit name to "submit2".

You should rather fix this issue on your side, as IE doesn't give CKEditor access to this form.

comment:4 Changed 4 years ago by alfonsoml

I haven't tested it, but if the reason is the duplicated input then it should be easy to fix because in that case what happens is that form.submit is an array with the two inputs, so checking if it's an array should be enough.

comment:5 Changed 4 years ago by narchos

It would be nice if it could be fixed. I know it may not be valid HTML but hey, most of the internet doesn't validate and I'm sure many CKEditor implementors out there will be caught unaware.

We are currently in the process of evaluating replacing lots of textareas with CKEditor. They exist in many old apps and the idea of changing the code to handle multiple submit names would be a big consideration when evaluating this great editor against the alternatives.

Keep up the great work!

comment:6 Changed 4 years ago by fredck

  • Status changed from closed to reopened
  • Resolution wontfix deleted

I also agree this is supposed to be a simple fix.

Changed 3 years ago by alfonsoml

Proposed patch

comment:7 Changed 3 years ago by alfonsoml

  • Owner set to alfonsoml
  • Status changed from reopened to review

The patch provides the basic protection to avoid the loading error, and it also tries to get the real form.submit() method by renaming existing elements for a moment.

Changed 3 years ago by alfonsoml

Revised patch

comment:8 Changed 3 years ago by alfonsoml

I think that it might be better to address just the bug report because after all, if the submit() method isn't available for us, I'm not sure that other code might be able to call it, so it doesn't matter if we try to do extra job in order to hook it.

If we want to address it, a first approach would be to add an "else" block to that condition and log a warning to the console so careful developers might notice it but don't bother users with it. "CKEditor: Unable to hook into the form.submit()"

comment:9 Changed 3 years ago by wwalc

  • Milestone set to CKEditor 3.5.1

comment:10 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

It's enough to just wrap the block with a try...catch.

comment:11 Changed 3 years ago by alfonsoml

  • Status changed from review_failed to review

The problem with a try..catch is that hides any error, so if there's something wrong inside that code we won't get any hint about it from now on.

The patch just addresses the reported bug and shouldn't create or hide any other problem.

comment:12 Changed 3 years ago by garry.yao

  • Status changed from review to review_passed

comment:13 Changed 3 years ago by alfonsoml

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [6300]

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