Opened 16 years ago
Closed 16 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.
- 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 (4)
Change History (11)
comment:1 Changed 16 years ago by
Owner: | set to Martin Kou |
---|---|
Status: | new → assigned |
comment:2 Changed 16 years ago by
Keywords: | Confirmed Safari added |
---|
comment:3 Changed 16 years ago by
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 16 years ago by
Attachment: | 3661.patch added |
---|
Changed 16 years ago by
Attachment: | 3661_debug.patch added |
---|
Changed 16 years ago by
Attachment: | cached.png added |
---|
Changed 16 years ago by
Attachment: | working.png added |
---|
comment:4 Changed 16 years ago by
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 16 years ago by
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 16 years ago by
Keywords: | Review+ added; Review? removed |
---|
comment:7 Changed 16 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed with [3633].
Click here for more info about our SVN system.
Seems to me there's a race condition in scriptloader.js - still investigating...