Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#3905 closed Bug (fixed)

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 (4)

unauthenticated content.jpg (849 bytes) - added by jonathanc 8 years ago.
3905.patch (5.1 KB) - added by Tobiasz Cudnik 8 years ago.
3905_2.patch (5.3 KB) - added by Tobiasz Cudnik 8 years ago.
3905_3.patch (4.8 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (22)

Changed 8 years ago by jonathanc

Attachment: unauthenticated content.jpg added

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0

comment:2 Changed 8 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

comment:3 Changed 8 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 8 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 8 years ago by Tobiasz Cudnik

Attachment: 3905.patch added

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Milestone: CKEditor 3.0CKEditor 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 8 years ago by Josh Nisly

Cc: fckeditor@… added

comment:7 Changed 8 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 8 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 8 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 8 years ago by Tobiasz Cudnik

Attachment: 3905_2.patch added

comment:10 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Owner: changed from Tobiasz Cudnik to Garry Yao
Status: assignednew
  • 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 8 years ago by Garry Yao

Attachment: 3905_3.patch added

comment:12 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed
Status: newassigned

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 8 years ago by Frederico Caldeira Knabben

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 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4583] at 3.1.x branch.

comment:15 Changed 7 years ago by tkrah

Milestone: CKEditor 3.1CKEditor 3.x
Resolution: fixed
Status: closedreopened
Version: SVN (CKEditor)3.2

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 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

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

comment:17 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Milestone: CKEditor 3.xCKEditor 3.1
Version: 3.23.0.2

comment:18 in reply to:  16 Changed 7 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 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy