Opened 9 years ago

Closed 9 years ago

#2791 closed Bug (fixed)

UI flashes when loading.

Reported by: Frederico Caldeira Knabben 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 (5)

overwrite_style.zip (885 bytes) - added by Frederico Caldeira Knabben 9 years ago.
Test page showing how a CSS file could overwrite inline styles.
2791.patch (1.3 KB) - added by Garry Yao 9 years ago.
slow_style_loading.patch (850 bytes) - added by Garry Yao 9 years ago.
2791.2.patch (1.3 KB) - added by Garry Yao 9 years ago.
2791_2.patch (1.3 KB) - added by Garry Yao 9 years ago.
Fix patch name

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by Frederico Caldeira Knabben

Attachment: overwrite_style.zip added

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

comment:1 Changed 9 years ago by Frederico Caldeira Knabben

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

Owner: set to Garry Yao
Status: newassigned

Changed 9 years ago by Garry Yao

Attachment: 2791.patch added

Changed 9 years ago by Garry Yao

Attachment: slow_style_loading.patch added

comment:3 Changed 9 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 9 years ago by Garry Yao

Keywords: Review? added

comment:5 Changed 9 years ago by Martin Kou

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

Attachment: 2791.2.patch added

Changed 9 years ago by Garry Yao

Attachment: 2791_2.patch added

Fix patch name

comment:6 Changed 9 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 9 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

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 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy