Ticket #3905 (closed Bug: fixed)

Opened 5 years ago

Last modified 4 years ago

Editor causes unauthenticated content warnings over SSL in FF 3.5

Reported by: jonathanc Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.1
Component: General Version: 3.0.2
Keywords: Confirmed Review+ Cc: fckeditor@…

Description

Firefox 3.5 is showing an unauthenticated content warning icon on https pages with editors. Such warnings look unprofessional and tend to scare users away.

The offending code is in ckeditor.js:

document.write(

'<script type="text/javascript" src="' + CKEDITOR.getUrl( '_source/core/loader.js' ) + '"></script>' );

Attachments

unauthenticated content.jpg (849 bytes) - added by jonathanc 5 years ago.
3905.patch (5.1 KB) - added by tobiasz.cudnik 5 years ago.
3905_2.patch (5.3 KB) - added by tobiasz.cudnik 5 years ago.
3905_3.patch (4.8 KB) - added by garry.yao 5 years ago.

Change History

Changed 5 years ago by jonathanc

comment:1 Changed 5 years ago by fredck

  • Milestone set to CKEditor 3.0

comment:2 Changed 5 years ago by tobiasz.cudnik

  • Owner set to tobiasz.cudnik
  • Status changed from new to assigned

comment:3 Changed 5 years ago by tobiasz.cudnik

  • Keywords Confirmed added

Problem exists in wysiwyg area plugin, which fills iframe using document.write & document.close. This causes FF to report unauthenticated content notice.

This seems very relevant to this gecko bug.

As for now i don't have idea how to deal with this. I'm testing different approaches to replace document.write.

comment:4 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added

Patch binds WYSIWYG area creation to iframe's onload. Works for all browser without workaround for FF and Opera. Custom domain is supported, but needs to be set 2 times.

Changed 5 years ago by tobiasz.cudnik

comment:5 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed
  • Milestone changed from CKEditor 3.0 to CKEditor 3.1

This approach would be good for several reasons. The most important thing is that we would not need to use a "bridge" to send the data to the iframe to be written, which makes the code much clearer.

Some things to be considered in the patch:

  • The "CKEDITOR._[ 'cke_htmlToLoad_' + editor.name ]" trick is not anymore needed. We can pass "data" directly to the createIFrame function at line 507.
  • The "onLoad" variable is not needed. The function can be passed directly to the on() call, and it's enough to call e.removeListener() to remove it at line 245.
  • The isCustomDomain variable has been removed from line 220, but isCustomDomain() is called twice in the patch, so it makes sense leaving that line intact and simply used the variable.

In any case, these changes are too risky to be done at this stage. We can work on it as soon as we release the 3.0.

comment:6 Changed 5 years ago by highjinx_53

  • Cc fckeditor@… added

comment:7 Changed 5 years ago by tobiasz.cudnik

I've implemented listed points and updated patch against newest trunk.

Bad news is that this doesn't seem to resolve unencrypted content notice on newest FF 3.5 (both win and linux).

comment:8 Changed 5 years ago by tobiasz.cudnik

FF 3.5 warning comes from HC detection in _bootstrap.js L25. It's about getComputedStyle particularly. Wondering if there's other way to determine is HC active in a browser.

comment:9 Changed 5 years ago by tobiasz.cudnik

Reason for this is use of "about:blank" hack as image source for browser other than IE 6. FF 3.5 parses this as "url(about:blank)" which is the reason for mixed-content SSL warning.

Changed 5 years ago by tobiasz.cudnik

comment:10 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added; Review- removed

comment:11 Changed 5 years ago by fredck

  • Owner changed from tobiasz.cudnik to garry.yao
  • Status changed from assigned to new
  • Keywords Review- added; Review? removed
  • Let's have a dedicated ticket for each thing at this point. Please open a new ticket for the wysiwyg data loading refactoring and provide a patch there.
  • For this ticket instead, it looks like the HC check fix is the only needed thing. The problem of using spacer.gif is that it makes this image file being downloaded, and we must avoid it. If there is no other way for it, let's include CKEDITOR.env.https in the check.

Changed 5 years ago by garry.yao

comment:12 Changed 5 years ago by garry.yao

  • Status changed from new to assigned
  • Keywords Review? added; Review- removed

Both issues ( document.write and about:blank ) are causing the 'partial authentication' error, so fixes to both places are needed.
Proposing of constructing image url with dataURI in supported browsers.

comment:13 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

Please commit it into the 3.1.x branch. We need to well test it over all browsers before releasing.

comment:14 Changed 5 years ago by garry.yao

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

Fixed with [4583] at 3.1.x branch.

comment:15 Changed 5 years ago by tkrah

  • Status changed from closed to reopened
  • Version changed from SVN (CKEditor) to 3.2
  • Resolution fixed deleted
  • Milestone changed from CKEditor 3.1 to CKEditor 3.x

Updating from 3.1 to 3.2 this is an issue again - don't know if its the same "cause", but firefox does complain again about unsecure content.

comment:16 follow-up: ↓ 18 Changed 5 years ago by garry.yao

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

We've already have #5359 opened for this, please keep update with that ticket.

comment:17 Changed 4 years ago by alfonsoml

  • Version changed from 3.2 to 3.0.2
  • Milestone changed from CKEditor 3.x to CKEditor 3.1

comment:18 in reply to: ↑ 16 Changed 4 years ago by delete_my_account

Replying to garry.yao:

We've already have #5359 opened for this, please keep update with that ticket.

I was a little bit puzzled, the correct ticket would be #5259

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