Opened 8 years ago

Closed 8 years ago

#3661 closed Bug (fixed)

Safari 3.2 cache issue

Reported by: Damian Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: IBM Confirmed Safari Review+ Cc:

Description

To reproduce:
Use Safari 3.2 on Windows. Ensure caching is not disabled.

  1. Open nightly Ajax example
  2. Click "Create Editor" (editor should open normally)
  3. Click "Close Editor" (editor should be closed normally)
  4. Repeat steps 2&3 to ensure the process is reproducible.
  5. On the same page, enter a new URL e.g. www.google.com and hit Enter.
  6. On the same page, enter the url back to the original Ajax example (e.g. http://nightly.ckeditor.com/3608/_samples/ajax.html) and hit Enter.
  7. Click "Create Editor"

At this point, the editor will not load until the page is reloaded. This error does not appear when cache is disabled in the browser

Attachments (4)

3661.patch (850 bytes) - added by Martin Kou 8 years ago.
3661_debug.patch (1.9 KB) - added by Martin Kou 8 years ago.
cached.png (363.4 KB) - added by Martin Kou 8 years ago.
working.png (410.3 KB) - added by Martin Kou 8 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:2 Changed 8 years ago by Martin Kou

Keywords: Confirmed Safari added

Seems to me there's a race condition in scriptloader.js - still investigating...

comment:3 Changed 8 years ago by Martin Kou

Keywords: Review? added

The bug is found to be caused by the timing of the onLoad function in scriptloader.js in Safari - Safari calls the function as soon as it is assigned to the <script> element when Safari finds the script is already cached.

Changed 8 years ago by Martin Kou

Attachment: 3661.patch added

Changed 8 years ago by Martin Kou

Attachment: 3661_debug.patch added

Changed 8 years ago by Martin Kou

Attachment: cached.png added

Changed 8 years ago by Martin Kou

Attachment: working.png added

comment:4 Changed 8 years ago by Martin Kou

I've talked to Garry a bit about this bug before lunch, but we were not too sure of the exact cause of this bug - all I got this morning was just a hypothesis, and thus the patch may be sub-optimal.

So to prove it's actually Safari calling onLoad too early, I've attached a debug patch which prints out debug messages. Also two screenshots - cached.png is when Safari is loading the scripts purely from cache and the editor didn't work; working.png is the usual case where the editor works.

So from the screenshots, when Safari is loading the scripts from cache, onLoad of a script tag would be called from inside or immediately after loadScript(). Since it is assumed onLoad shouldn't be called immediately in our logic, Safari's execution order is breaking the script loader's logic.

comment:5 Changed 8 years ago by Martin Kou

There's still one concern in the patch here...

Now that the setTimeout() is executed for every browser, it may negatively impact performance. But I'm not sure if it actually happens.

On the other hand, if I add an if statement to check for Safari, I'd have increased the core code size unnecessarily - the patch works in other browsers as it is.

So the current patch is choosing the smaller code size option.

comment:6 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:7 Changed 8 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [3633].

Click here for more info about our SVN system.

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