Ticket #3661 (closed Bug: fixed)
Safari 3.2 cache issue
| Reported by: | damo | Owned by: | martinkou |
|---|---|---|---|
| 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.
- Open nightly Ajax example
- Click "Create Editor" (editor should open normally)
- Click "Close Editor" (editor should be closed normally)
- Repeat steps 2&3 to ensure the process is reproducible.
- On the same page, enter a new URL e.g. www.google.com and hit Enter.
- 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.
- 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
Change History
comment:1 Changed 4 years ago by martinkou
- Owner set to martinkou
- Status changed from new to assigned
comment:2 Changed 4 years ago by martinkou
- Keywords Confirmed Safari added
Seems to me there's a race condition in scriptloader.js - still investigating...
comment:3 Changed 4 years ago by martinkou
- 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.
comment:4 Changed 4 years ago by martinkou
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 4 years ago by martinkou
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:7 Changed 4 years ago by martinkou
- Status changed from assigned to closed
- Resolution set to fixed
Fixed with [3633].
Click here for more info about our SVN system.
