Ticket #2791 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

UI flashes when loading.

Reported by: fredck Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Firefox Review? Cc:

Description

In Firefox, it's possible to quickly see the editor and the dialog rendered without skin even before the skin CSS is loaded. Then, once the CSS is ready, it redraws the UI in the right way. Not checked, but the same thing may happen with the contents.

There should be a way to hold the rendering of any UI element until the CSS is loaded.

Attachments

overwrite_style.zip (885 bytes) - added by fredck 5 years ago.
Test page showing how a CSS file could overwrite inline styles.
2791.patch (1.3 KB) - added by garry.yao 5 years ago.
slow_style_loading.patch (850 bytes) - added by garry.yao 5 years ago.
2791.2.patch (1.3 KB) - added by garry.yao 5 years ago.
2791_2.patch (1.3 KB) - added by garry.yao 5 years ago.
Fix patch name

Change History

Changed 5 years ago by fredck

Test page showing how a CSS file could overwrite inline styles.

comment:1 Changed 5 years ago by fredck

I've attached a test page which uses the !important directive in the CSS, overwriting inline styles. This could be a solution for us, hiding UI elements on code and overwriting them with the skin CSS.

comment:2 Changed 5 years ago by garry.yao

  • Status changed from new to assigned
  • Owner set to garry.yao

Changed 5 years ago by garry.yao

Changed 5 years ago by garry.yao

comment:3 Changed 5 years ago by garry.yao

It seems that:

So I make a simple workaround by using style rule priority to override. Use the attached slow_style_loading.patch for test the patch locally.

comment:4 Changed 5 years ago by garry.yao

  • Keywords Review? added

comment:5 Changed 5 years ago by martinkou

I'm in favor of Garry's approach there, !important is something to be avoided if possible.

Garry's patch, however, obviously wouldn't work - Garry commented out his own code. :)

Possible concerns with Garry's patch:

  1. If I have multiple editor instances within the same page, then I'll have more than one style tag with the same contents. (e.g. replacebycode.html sample) That doesn't cause any immediate errors, but seems wasteful to me.
  2. It's more correct to include style tags inside <head>. In v2 we used to add CSS styles online like this (it's actually split into fcktools_ie.js and fcktools_gecko.js)...
    if ( document.createStyleSheet )
    {
      var ss = document.createStyleSheet('');
      ss.cssText = cssStyles;
    }
    else
    {
      var head = document.getElementsByTagName('head')[0];
      var ss = document.createElement('style');
      ss.appendChild(document.createTextNode(cssStyles));
      head.appendChild(ss);
    }
    

Fixing point 1 and 2 above would increase the code size a bit though, and Garry's current approach (with his added line in theme.js uncommented) doesn't seem to raise any errors or warnings anyway.

Changed 5 years ago by garry.yao

Changed 5 years ago by garry.yao

Fix patch name

comment:6 Changed 5 years ago by garry.yao

Thanks Martin, that's a big mistake and your approach is ideal, seems now multiple instances should always be considered.

comment:7 Changed 5 years ago by fredck

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

I've fixed it with [3015] to avoid delay on the release. It's based on Garry's approach. I've added the same trick to the dialog also.

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