Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#8602 closed Bug (fixed)

Unnecessary calls to images without baseHref

Reported by: pm_ele Owned by: fredck
Priority: Normal Milestone: CKEditor 3.6.3
Component: General Version:
Keywords: HasPatch IE Cc:

Description

When displaying images using relative urls and config.baseHref, everytime I switch to wysiwyg mode there is a call made to the image using only the relative url and totally ignoring the baseHref. Obviously this fails into a 404 (or some other error code depending on the environnement). The image is still displayed, but there is also this unnecessary 404 call. Please note also that if you're testing this on local static files (i mean something like C:/some_directories/mypage.html) the call won't show up on Firebug, you will need to run it on a server like wamp.

You can reproduce it easily on the online demo with Firebug :

  • go to http://ckeditor.com/demo
  • go to source mode
  • remove all source and paste only the following :
     <img src="logo3w.png"/>
    
  • type in Firebug console :
    CKEDITOR.instances.editor1.config.baseHref = 'http://www.google.fr/images/srpr/';
    
  • clear all firebug console and network panel
  • hit the wysiwyg mode

you should see the Google logo, and a 404 showing up in both console et network panels.

I've located the where the call is made, it's in plugins/htmldataprocessor/plugin.js, in the toHtml function :

    // Call the browser to help us fixing a possibly invalid HTML
    // structure.
    var div = new CKEDITOR.dom.element( 'div' );
    // Add fake character to workaround IE comments bug. (#3801)
    div.setHtml( 'a' + data );
    data = div.getHtml().substr( 1 );

The data is submitted to the browser, causing it to interpret the <img> tags without taking the config.baseHref into account.

I've found a solution by using a regexp on the data : I turn all the src attributes into data attributes and back just before and after submitting the data to the browser. I'm not sure it's best way but I'm using it on the ckeditor on my website and it works fine. I can't make a patch file but here's how I've altered the code :

    //Call the browser to help us fixing a possibly invalid HTML
    // structure.        
    var fakeSrcAttribute = 'data-cke-faked-src';
    data = data.replace(/(<img[^>]+)src(\s*=)/ig, '$1' + fakeSrcAttribute + '$2');        
    var div = new CKEDITOR.dom.element( 'div' );
    // Add fake character to workaround IE comments bug. (#3801)
    div.setHtml( 'a' + data );
    data = div.getHtml().substr( 1 );
    data = data.replace(new RegExp('(<img[^>]+)' + fakeSrcAttribute + '(\\s*=)', 'ig'), '$1src$2');

Attachments (2)

8602.patch (8.3 KB) - added by fredck 4 years ago.
8602_2.patch (8.8 KB) - added by fredck 4 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 5 years ago by pm_ele

  • Keywords IE7 added

I just noticed : the fix I'm suggesting also solves much bigger bug I had in IE7.

In IE7, images with relatives urls and baseHref were simply not displayed. It seems that submitting the data to the browser caused IE7 to try to replace the relative urls in src attributes by incorrect absolute urls. The suggested fix solves that bug too.

comment:2 Changed 4 years ago by j.swiderski

  • Keywords HasPatch added; baseHref 404 images IE7 removed
  • Status changed from new to confirmed

This part about 404 is actually a duplicate of #6368. Yet I will not close it as it contains potential fix to perhaps two issues.

@pm_ele Could you explain me this part with IE7? I have seen many tickets about IE7, baseHref and relative URL but could not reproduce any.

Take this URL: /intl/en_ALL/images/logo.gif and this baseHref config.baseHref = 'http://www.google.com/intl/en_ALL/images/'; for example. They work.

@pm_ele is it possible to reproduce this issue in CKEditor, without any webapp? If so could you provide us with scenario to do so?

comment:3 Changed 4 years ago by abcito

Awesome fix

comment:4 Changed 4 years ago by j.swiderski

@abcito just like I have written in #5042.

Does this bug occur in plain CKEditor downloaded form our site or only when included in your webapp?

Any chance for a TC here?

comment:5 Changed 4 years ago by fredck

  • Keywords IE added

I'm able to reproduce this issue with IE9+Compat. It has nothing to do with #6368.

Changed 4 years ago by fredck

comment:6 Changed 4 years ago by fredck

  • Owner set to fredck
  • Status changed from confirmed to review

The suggested fix makes sense, but it can be done in a a single pass.

The attached patch includes also a minor new feature. Note that the numbers on CKEDITOR.rdn will be optimized by the packager.

comment:7 Changed 4 years ago by garry.yao

  • Status changed from review to review_failed

If this patch is to be considered, [7379] could be reverted.

Changed 4 years ago by fredck

comment:8 Changed 4 years ago by fredck

  • Status changed from review_failed to review

True... killing two birds with one stone so ;)

comment:9 Changed 4 years ago by fredck

  • Milestone set to CKEditor 3.6.3

comment:10 Changed 4 years ago by garry.yao

  • Status changed from review to review_passed

comment:11 Changed 4 years ago by fredck

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

Fixed with [7413].

comment:12 follow-up: Changed 4 years ago by alfonsoml

Just a minor note: the example says alert( CKEDITOR.rnd ); but the used name is "rdn" instead.

Personally, I think that "rnd" might be better because rdn isn't an abbreviation of random.

comment:13 in reply to: ↑ 12 Changed 4 years ago by fredck

Replying to alfonsoml:

Nice observation. In fact, the example showed what I wanted to have, but I've spread the typo everywhere.

Renamed with [7417].

comment:14 Changed 4 years ago by garry.yao

http://ckeditor.t/tt/4475/1.html The above tt is regressed, post fixed with [7418].

comment:15 Changed 4 years ago by abcito

Hmm applied the patch and I have an image

<img alt="masthead" src="static/assets/banner900x120.gif" style="width:900px; height:120px;" /> (in source)

toggled to WYSIWYG then back to source and I get this

<img alt="mastheadsrc=&quot;static/assets/banner900x120.gif&quot;" src="static/assets/banner900x120.gif" style="width: 900px; height: 120px" />

thoughts?

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