Update notebook to utilize pure javascript JSmol for default live 3-D

[gutow]

The notebook needs updates to utilize a pure javascript version of Jmol by default for live 3-D. This includes changes to make the notebook compatible with the newer jquery-1.9 (some things used in the notebook have been deprecated).

Jmol is cross compiled to java and javascript. Because so many browser/OS combinations now actively try to avoid using java applets, it is necessary to have a pure javascript alternative. This solution provides all the functionality of Jmol, but as expected since javascript is interpreted rather than bytecode ("~3/4" of the way to compiled) the performance when interacting with the plot is slower.

This first implementation removes the user tools that have appeared to the right of the 3-D plot as they need to be rewritten as well.

One added feature is a "load 3-D live" check box that can be used to make a worksheet load 3-D plots live when the worksheet is opened. For worksheets with a single interact or only 1-2 plots this should be OK.

This requires installation of an updated Jmol.spkg. See ticket #17020. Will require a new sagenb release: [INSERT HERE]

How to test:

1. Update Sage with #17020
2. Download the repackaged Jmol from the link provide in #17020 and save the package in the "upstream" directory of your sage install.
3. run sage -f jmol to install the new jmol.
4. If you don't have it yet, update Sage with #16911 or just use this branch that Volker has merged with that one.
5. Same with #16396 if you want to build doc correctly.
6. Update Sage with the branch here
7. Put the new upstream package into SAGE_ROOT/upstream
8. Do sage -f sagenb-0.11.0
9. Test!
10. For testing, #17170 is also advised, since without it display in the notebook does not work properly.

Depends on #17020

Upstream: Reported upstream. Developers acknowledge bug.

CC: @kcrisman @novoselt @strogdon @jasongrout @fchapoton

Component: notebook

Keywords: Jmol 3D 3-D

Author: Jonathan Gutow, Volker Braun

Branch: a4c94cd

Reviewer: Steven Trogdon, Karl-Dieter Crisman, Jonathan Gutow

Issue created by migration from https://trac.sagemath.org/ticket/16004

[vbraun]

comment:94

Calling GraphicsFile.launch_viewer is only for commandline use.

[kcrisman]

comment:95

Calling GraphicsFile.launch_viewer is only for commandline use.

I believe you, but at any rate something is going wrong. Do you get things to show up without show(...)?

[vbraun]

comment:96

Yes, can reproduce. Its fallout from the IPython notebook, will fix it at #17170

[kcrisman]

comment:97

Yes, can reproduce. Its fallout from the IPython notebook, will fix it at #17170

As suspected, thanks. The other big thing is then figuring out what the heck is going wrong with my Safari - it is an older one that github doesn't like, true, but our javascript hasn't changed that much! The other things hopefully can be fixed fairly easily for the new cell logic.

[kcrisman]

comment:98

As suspected, thanks. The other big thing is then figuring out what the heck is going wrong with my Safari - it is an older one that github doesn't like, true, but our javascript hasn't changed that much!

I must be going crazy, because now it is working again. ???

[kcrisman]

comment:99

• Shift-click is still broken, because Volker just took your commits but didn't fix them. See https://github.com/kcrisman/sagenb/commit/4fe29c8f7ab6ae5a44bf9a51872b3008ee4d18a2 for a commit that (perhaps with minor modifications) fix this - I think I actually got it from Jonathan but perhaps modified it, that was a while ago now.

Now all of a sudden this is working after all, which makes sense because https://github.com/vbraun/sagenb/commit/f9760a8635a63511731e340046711b1a7288de1c is there.

• The new cell bar, when it is above a text area, does not have the icons. In such cases, it also does not work.

I think this is probably due to not having changed sagenb/data/sage/html/notebook/text_cell.html in the relevant commits https://github.com/vbraun/sagenb/commit/8fe8371b93112d29a4096307d5807d736c41e415 and https://github.com/vbraun/sagenb/commit/2757d2c6b488feeae2a3f97fffdd09f7ffe5ef01 Naturally, one would also have to do the thing to fix shift-click in that spot as well.

[gutow]

Description changed:

--- 
+++ 
@@ -14,5 +14,4 @@
 2. Download the repackaged Jmol from the link provide in #17020 and save the package in the "upstream" directory of your sage install.
 3. run `sage -f jmol` to install the new jmol.
 4. Clone the `new_jmol` branch from the PR to anywhere convenient on your system: `git clone https://github.com/vbraun/sagenb.git -b new_jmol`
-5. Apply the three attached patches to this new sagenb repository.
-6. From within the sagenb repository run `sage -python setup.py install`
+5. From within the sagenb repository run `sage -python setup.py install`

[gutow]

comment:101

I pushed my three patches to the repository, so those should no longer need to be manually installed. I am looking into Karl-Dieter's problems. I hadn't doubled checked the back compatible stuff.

[gutow]

comment:102

Testing in my version, which is now synced to Volker's repository:

1. shift-click works everywhere in FF and chromium (linux chrome)
2. cntrl-click works everywhere in FF and chromium
3. clicking on the two new cell icons works everywhere in FF and chromium
4. right-click JSmol popup works in FF and chromium
5. right-click Jmol popup works in FF, cannot test chromium as it does not support java plugin anymore.
6. The evaluate button always shows up for me. Occasionally, I have seen issues with interacts covering the evaluate button, but it reappears after clicking around. I have not been able to reproduce it reliably.

Although I have a lot of Macs, I do not think I have an old enough safari to reproduce the "old" problems on. For the above items I am not seeing problems so am unable to do anything to fix them. Please verify which ones are still problems.

[gutow]

comment:103

Replying to @kcrisman:

• onchange="3D_use_java_check(this.checked);" looks nice, except there is no such function in sagenb/data/sage/js/notebook_lib.js, unlike for pretty_print_check and live_3D_check. Does it need to be added there? If not and things still work, then that line should be removed.

The function is now in jmol_lib.js. I did not put it in the notebook_lib.js because this function does not need to talk to the server. Since the choice to use java needs to be decided in realtime by the user because of how browser and computer dependent it is, the setting is not a sticky.

[kcrisman]

comment:104

Testing in my version, which is now synced to Volker's repository:

Thanks for this!

Although I have a lot of Macs, I do not think I have an old enough safari to reproduce the "old" problems on. For the above items I am not seeing problems so am unable to do anything to fix them. Please verify which ones are still problems.

Can you confirm that the things that work above for you on FF/Chromium also work for some version >= 6 of Safari? I think that is what you are saying.

[kcrisman]

comment:105

Let me try to summarize what probably still remains, assuming I also have luck with the most up-to-date repo when I can get to this Mondayish. These are obviously fairly minor compared to the display hook thing (thanks for fixing that so quickly!), but they worked in my branch.

•   removal (or rather, unremoval)
• hardcoded Jmol version is still 14.0.13
• The new cell bar, when it is above a text area, does not work and has no icons. It should also have the custom ctrl/shift-click code from other patches added to it.
• The new cell bar is not quite at the right indentation level (div?) at the very end of the worksheet
• Color of new cell bar should go back.
• The js in comment:91 should be fixed so that it only applies to the pop out introspection. You can check this by clicking the "pop to own window" and it won't do what it is supposed to. By the way, what is ctrl-click supposed to do on a new cell bar? I must have forgotten this feature!

[gutow]

comment:106

Replying to @kcrisman:

Let me try to summarize what probably still remains, assuming I also have luck with the most up-to-date repo when I can get to this Mondayish. These are obviously fairly minor compared to the display hook thing (thanks for fixing that so quickly!), but they worked in my branch.

•   removal (or rather, unremoval)

This is just for layout with the checkboxes. I didn't find having it missing making an obvious layout difference, but we can put it back in.

• hardcoded Jmol version is still 14.0.13

I will have to dig into Jmol for this. It is supposed to pick up the version, which can be shown to be correct in the console by selecting the "State". This is an upstream bug, although I might be able to patch it here.

• The new cell bar, when it is above a text area, does not work and has no icons. It should also have the custom ctrl/shift-click code from other patches added to it.

Now I understand. We need to add a new cell bar above new rich text boxes. I completely missed that.

• The new cell bar is not quite at the right indentation level (div?) at the very end of the worksheet

hmmm, I'll look at that.

• Color of new cell bar should go back.

Are you saying the highlighting doesn't disappear when you stop hovering over the new cell bar? That may be a browser bug. I will look at it again.

• The js in comment:91 should be fixed so that it only applies to the pop out introspection. You can check this by clicking the "pop to own window" and it won't do what it is supposed to.

Not sure what the "pop to own window" is. I know we used to have a link for that, but it should be gone along with all the fancier controls, they still need rewriting.

By the way, what is ctrl-click supposed to do on a new cell bar? I must have forgotten this feature!

shift-click = new rich text cell cntrl-click = new calculation cell

[kcrisman]

comment:107

• The new cell bar, when it is above a text area, does not work and has no icons. It should also have the custom ctrl/shift-click code from other patches added to it.

Now I understand. We need to add a new cell bar above new rich text boxes. I completely missed that.

Ok. I should point out that there IS a new cell bar, but it doesn't have the icons and doesn't do anything if you click on it. Try a worksheet with some already-existing text cells and some computation cells, and you'll see what I mean when you hover in various places.

• Color of new cell bar should go back.

Are you saying the highlighting doesn't disappear when you stop hovering over the new cell bar? That may be a browser bug. I will look at it again.

No, I mean that you changed the color of the bars for some reason. See my previous comments. If you look at my branch you can see I remove your mysterious change to cyan and put it back to the usual color.

• The js in comment:91 should be fixed so that it only applies to the pop out introspection. You can check this by clicking the "pop to own window" and it won't do what it is supposed to.

Not sure what the "pop to own window" is. I know we used to have a link for that, but it should be gone along with all the fancier controls, they still need rewriting.

Nothing to do with jmol. If you do binomial? or something like that, you will see the "introspection" documentation; there should be a link to click to "pop out" that into a separate window, and there are a few subtle things about that that don't work if you don't change that. Look at the specific code I'm talking about - you changed it when you updated to modern jQuery, but unfortunately put in commas that didn't belong. Just take my code :-)

By the way, what is ctrl-click supposed to do on a new cell bar? I must have forgotten this feature!

shift-click = new rich text cell cntrl-click = new calculation cell

Huh. Doesn't also just plain old clicking (no other keys) make a new calculation cell? Or maybe I'm misunderstanding something here.

[gutow]

comment:108

Replying to @kcrisman:

Ok. I should point out that there IS a new cell bar, but it doesn't have the icons and doesn't do anything if you click on it.

Yep, it didn't do anything because it had the old non-jquery compatible code in it. I have a fix for that and will commit it before the end of the weekend. I can also tell you that the end of sheet new cell bar is indented differently because it is not part of the worksheet cell list. It has the same indentation as rich text cells. If it is really bothersome, I can place the icons over a little, but we do not want that cell to be part of the cell list.

While working on this I also realized there is no way to delete rich text cells. Any thoughts on whether that is something we want? If so, it should be its own ticket.

No, I mean that you changed the color of the bars for some reason. See my previous comments. If you look at my branch you can see I remove your mysterious change to cyan and put it back to the usual color.

hmm, I'm not sure why I changed it. Maybe I couldn't find the right color. We certainly can pull it from your branch.

Nothing to do with jmol. If you do binomial? or something like that, you will see the "introspection" documentation; there should be a link to click to "pop out" that into a separate window, and there are a few subtle things about that that don't work if you don't change that. Look at the specific code I'm talking about - you changed it when you updated to modern jQuery, but unfortunately put in commas that didn't belong. Just take my code :-)

OK, I'm not sure why that comma is there either.

Huh. Doesn't also just plain old clicking (no other keys) make a new calculation cell? Or maybe I'm misunderstanding something here.

Looking at the code any click other than shift-click creates a new compute cell. This kind of confusion is why I like the little icons.:)

[gutow]

comment:109

Pushed following fixes to Volker's git repository: Reverted new cell div hover color to old color Added   before text of display control checkboxes Made add cell work before above rich text cells Removed commas that were messing up jquery binding to certain items

Test and let me know what else you find...

[gutow]

comment:110

Replying to @kcrisman:

• hardcoded Jmol version is still 14.0.13

This is a bug in 14.2.4. It appears to be fixed in 14.2.7. We can make a new spkg. Doesn't look like the static image for java fix is in 14.2.7 yet, so we will have to wait on that.

[gutow]

comment:111

Pushed a commit to #17020 that should fix the version number issue in the popup menus, independent of Jmol/JSmol version.

[kcrisman]

comment:112

Yep, it didn't do anything because it had the old non-jquery compatible code in it. I have a fix for that and will commit it before the end of the weekend.

Great, I'll look at that today.

I can also tell you that the end of sheet new cell bar is indented differently because it is not part of the worksheet cell list. It has the same indentation as rich text cells. If it is really bothersome, I can place the icons over a little, but we do not want that cell to be part of the cell list.

Got it, that makes sense.

While working on this I also realized there is no way to delete rich text cells. Any thoughts on whether that is something we want? If so, it should be its own ticket.

Hmm, you can't delete them, it's true, but save+quit+open actually gets rid of such cells if they are empty, due to the way sagenb handles that internally, no worries.

Looks like you got the other stuff on my list (or explained it). I would personally not worry about waiting for the static image for java fix if the js version is the default...

[kcrisman]

comment:113

I'm going to continue discussion on the pull request since we are so close now.

[kcrisman]

Description changed:

--- 
+++ 
@@ -15,3 +15,5 @@
 3. run `sage -f jmol` to install the new jmol.
 4. Clone the `new_jmol` branch from the PR to anywhere convenient on your system: `git clone https://github.com/vbraun/sagenb.git -b new_jmol`
 5. From within the sagenb repository run `sage -python setup.py install`
+
+For testing, #17170 is also advised, since without it display in the notebook does not work properly.

[kcrisman]

Reviewer: Steven Trogdon, Karl-Dieter Crisman, Jonathan Gutow

[kcrisman]

comment:115

I think we really need more reviewers now at the pull request.

On a likely related note, I think the following is due to the changes here. Any ideas?

• If you make a standard 2d plot like plot(1-x,(x,0,1)), evaluate, and then go back to that same cell and change it to something else like plot(1-x^2,(x,0,1)), the graphic changes. But:
• Try doing a Jsmol, and then try changing that notebook cell and re-evaluating. If you don't have the "Load 3D Live" checkbox clicked, you'll think you changed it - but you didn't! As soon as you make it live, it's the exact same one as before. If you have the live checkbox clicked (this refers to #16004, for those who come new to this) then this is quite obvious.

If it helps, the filenames are definitely changing:

$ ls .sage/sage_notebook.sagenb/home/admin/264/cells/82
sage0-size500-155928955.jmol.zip  sage0-size500.jmol
$ ls .sage/sage_notebook.sagenb/home/admin/264/cells/82
sage0-size500-938371752.jmol.zip  sage0-size500.jmol
$ ls .sage/sage_notebook.sagenb/home/admin/264/cells/82
sage0-size500-522166844.jmol.zip  sage0-size500.jmol
$ ls .sage/sage_notebook.sagenb/home/admin/264/cells/82
sage0-size500-996347389.jmol.zip  sage0-size500.jmol

but I suspect that something is not being overwritten somewhere.

[gutow]

comment:116

No ideas on the above, since I do not see the problem in my build. I am running the latest commits, but am not sure from the discussions on the pull list if you have some other commits from different branches.

[gutow]

comment:117

Replying to @kcrisman:

On a likely related note, I think the following is due to the changes here. Any ideas?

but I suspect that something is not being overwritten somewhere.

Is it possible you are running into overzealous caching? The change in file name should fix that. Is the file name changing in the html on your page? If not your browser is not updating the page.

[gutow]

comment:118

Ha! I just got it to happen...Now if I can only figure out what I did!

[gutow]

comment:119

OK, I'm almost sure it is a caching issue. I think we are being bitten by something I've wanted to rewrite for a while. In addition to the data file, Sage writes a custom Jmol script to launch the display. I think we could avoid using it, but I've tried to change as little as possible to maintain backwards compatibility. Anyway, I don't think this is a new bug. The script is not given a unique name, so we are picking up the one from the cache. I will see if the simple fix of providing a unique name will work. Ultimately that script file should go and we should just make a ".jmol" file.

[kcrisman]

comment:120

OK, I'm almost sure it is a caching issue.

I just tried this as well and I think I agree with you. But it's completely pervasive for me, even interacts are now no longer working properly. I get it now consistently in Safari, FF, but not Chrome. At least, not on the js, though on the java it seems to still happen. (Clearing cache in Chrome is particularly annoying with the Java because it also logs me out of Sage as you have to clear plugins and cookies together, can't only clear plugins.)

Anyway, I don't think this is a new bug.

Perhaps, but I think I would have noticed this if it was common before. I can't get it to not happen in FF and Safari right now :(

The script is not given a unique name, so we are picking up the one from the cache. I will see if the simple fix of providing a unique name will work.

Well, that would be nice. Sometimes programming even does have nice solutions - let's hope. Thanks very much for looking into this.

[kcrisman]

comment:121

More info. Clicking on the js console gives

FileManager opening 1 http://localhost:8080/home/admin/302/cells/1/sage0-size500.jmol
FileManager opening 3 http://localhost:8080/home/admin/302/cells/1/sage0-size500-184635879.jmol.zip|SCRIPT
FileManager opening 1 http://localhost:8080/home/admin/302/cells/1/sage0-size500-184635879.jmol.zip
The Resolver thinks Xyz
reading 10 atoms
ModelSet: haveSymmetry:false haveUnitcells:false haveFractionalCoord:false
1 model in this collection. Use getProperty "modelInfo" or getProperty "auxiliaryInfo" to inspect them.
Default Van der Waals type for model set to Babel
10 atoms created
ModelSet: autobonding; use autobond=false to not generate bonds automatically
no data
pmesh obj_729730 "obj_729730.pmesh";
reading isosurface data from http://localhost:8080/home/admin/302/cells/1/sage0-size500-184635879.jmol.zip|obj_729730.pmesh
FileManager opening 3 http://localhost:8080/home/admin/302/cells/1/sage0-size500-184635879.jmol.zip|obj_729730.pmesh
FileManager opening 1 http://localhost:8080/home/admin/302/cells/1/sage0-size500-184635879.jmol.zip

But

$ cd .sage/sage_notebook.sagenb/home/admin/302/cells/1
$ ls
sage0-size500-508165167.jmol.zip    sage0-size500.jmol

So the old 1846... isn't even in the directory! Is it in the .jmol file? What role does that file even play?

---

Another thing I'm noticing is that, in the past, I thought one could only discard changes since the last cell evaluation, which sort of automatically saved. But for some reason now "discard changes" seems to discard ALL changes since the last save. I can't see now how any of this could be responsible for that, but I am going to have to confirm that this is either prior behavior (which I don't buy) or unrelated (which I hope).

[kcrisman]

comment:122

Another thing I'm noticing is that, in the past, I thought one could only discard changes since the last cell evaluation, which sort of automatically saved. But for some reason now "discard changes" seems to discard ALL changes since the last save. I can't see now how any of this could be responsible for that, but I am going to have to confirm that this is either prior behavior (which I don't buy) or unrelated (which I hope).

Yikes, this is actual behavior. I mean, it's consistent, but scary without autosave... I don't know how I ever didn't notice this before. Save often!

[kcrisman]

comment:123

Jonathan, try this.

diff --git a/sagenb/notebook/cell.py b/sagenb/notebook/cell.py
index 7c322c9..d0ce526 100755
--- a/sagenb/notebook/cell.py
+++ b/sagenb/notebook/cell.py
@@ -2350,7 +2350,7 @@ class Cell(Cell_generic):
             <div id="loadJmol" style="display:none;">{id}</div>
             <div id="sage_jmol_size_{id}" style="display:none;">{size}</div>
             <div id="sage_jmol_img_{id}" style="display:none;">{image_name}.png?{timestamp}</div>
-            <div id="sage_jmol_script_{id}" style="display:none;">{filename}</div>
+            <div id="sage_jmol_script_{id}" style="display:none;">{filename}?{timestamp}</div>
             <div id="sage_jmol_server_url_{id}" style="display:none;">{callback}</div>
             <div id="sage_jmol_status_{id}" style="display:none;">notActivated</div>
         </div>

I think that the source of this is this commit where the ?time.time() was inadvertently removed in an otherwise-useful refactoring.

[kcrisman]

comment:124

Jonathan, try this.

Yeah, this simple change back to your original code intent reliably fixes the problem for me (undoing it causes the problem again).

Aagh! But it seems to break Chrome for some reason.

Error connecting to server: /home/admin/302/jsmol?call=getRawDataFromDatabase&database=_&query=http%3A%2F%2Flocalhost%3A8080%2Fhome%2Fadmin%2F302%2Fcells%2F1%2Fsage0-size500.jmol%3F1413900723.07&encoding=base64

and an error message logged

exceptions.IOError: [Errno 2] No such file or directory: u'/Users/.../.sage/sage_notebook.sagenb/home/admin/302/cells/1/sage0-size500.jmol1413900723.07

So maybe you are right and we have to actually change the name of the script as such. But I don't want to do that in the Sage library for 3d graphics any more, so I'll try to figure out if we can dynamically do it in the file cell.py.

[kcrisman]

comment:125

Okay, this much more hackish code does seem to work properly, including dealing with reloading pages with old files, etc.

diff --git a/sagenb/notebook/cell.py b/sagenb/notebook/cell.py
index 7c322c9..20ea9c1 100755
--- a/sagenb/notebook/cell.py
+++ b/sagenb/notebook/cell.py
@@ -2333,8 +2333,8 @@ class Cell(Cell_generic):
         #
         # So we need to prepend the worksheet URL, in order
         # for the zip to be accessed correctly.
+        jmol_name = os.path.join(self.directory(), F)
         if self.worksheet().docbrowser():
-            jmol_name = os.path.join(self.directory(), F)
             with open(jmol_name, 'r') as f:
                 jmol_script = f.read()
             jmol_script = jmol_script.replace(
@@ -2343,8 +2343,12 @@ class Cell(Cell_generic):
             with open(jmol_name, 'w') as f:
                 f.write(jmol_script)

+        F_time = F.replace('.jmol','-{}time.jmol'.format( time.time() ))
+        jmol_name_time = os.path.join(self.directory(), F_time)
+        shutil.copy2(jmol_name, jmol_name_time)
+
         image_name = os.path.join(self.url_to_self(),'.jmol_images',F)
-        script_name = os.path.join(self.url_to_self(), F)
+        script_name = os.path.join(self.url_to_self(), F_time)
         return textwrap.dedent("""
         <div id="sage_jmol_{id}" class="3DPlotDiv">
             <div id="loadJmol" style="display:none;">{id}</div>
@@ -2423,6 +2427,8 @@ class Cell(Cell_generic):
                 pass # obj data
             elif F.endswith('.svg'):
                 images.append('<embed src="%s" type="image/svg+xml" name="emap">' % url)
+            elif F.endswith('time.jmol'):
+                pass
             elif F.endswith('.jmol'):
                 images.append(self._jmol_files_html(F))
                 jmolimagebase = F

But I'd much rather hear that there is a way to make Chrome like the ? syntax for the script and not just the graphic.

[kcrisman]

comment:126

Here is at least one piece of the problem, in the file sagenb/flask_version/worksheet.py:

filename = match.group('filename')
filename = secure_filename(filename) # never trust input

This removes the ? without the rest.

Interestingly, one reason this happens at all is because only Chrome goes through this URI

@worksheet_command('jsmol')
def worksheet_jsmol_data(worksheet):

as far as I can tell. I have no idea why the others don't.

Anyhow, maybe this updated version of the first diff would work.

diff --git a/sagenb/flask_version/worksheet.py b/sagenb/flask_version/worksheet.py
index 40c3d54..fd0dcf2 100644
--- a/sagenb/flask_version/worksheet.py
+++ b/sagenb/flask_version/worksheet.py
@@ -695,6 +695,7 @@ def worksheet_jsmol_data(worksheet):
             return current_app.message(_('Invalid JSmol query: ' + query))
         cell_id = match.group('cell_id')
         filename = match.group('filename')
+        filename = filename.rsplit('?',1)[0] # appended query is only for cache busting
         filename = secure_filename(filename)   # never trust input
         filename = os.path.join(worksheet.cells_directory(), cell_id, filename)
         with open(filename, 'r') as f:
diff --git a/sagenb/notebook/cell.py b/sagenb/notebook/cell.py
index 7c322c9..d0ce526 100755
--- a/sagenb/notebook/cell.py
+++ b/sagenb/notebook/cell.py
@@ -2350,7 +2350,7 @@ class Cell(Cell_generic):
             <div id="loadJmol" style="display:none;">{id}</div>
             <div id="sage_jmol_size_{id}" style="display:none;">{size}</div>
             <div id="sage_jmol_img_{id}" style="display:none;">{image_name}.png?{timestamp}</div>
-            <div id="sage_jmol_script_{id}" style="display:none;">{filename}</div>
+            <div id="sage_jmol_script_{id}" style="display:none;">{filename}?{timestamp}</div>
             <div id="sage_jmol_server_url_{id}" style="display:none;">{callback}</div>
             <div id="sage_jmol_status_{id}" style="display:none;">notActivated</div>
         </div>

Try one or all of these out.

[kcrisman]

comment:127

For positive review, I think here is what we still need.

• Positive review for #17020
• Positive review for #17170, thought this is somewhat orthogonal
• Merging my minor css change stuff at https://github.com/vbraun/sagenb/pull/1
• Some fix to this last cache busting issue

Then I think we need to really push it into a new package for upstream so that it can be tested thoroughly. Depending on where Volker is in the release process, maybe the first beta of the next one, or in the current beta series.

[gutow]

comment:128

(+1) This last patch works for me on FF/linux and is reasonably simple.

Anyhow, maybe this updated version of the first diff would work.

diff --git a/sagenb/flask_version/worksheet.py b/sagenb/flask_version/worksheet.py
index 40c3d54..fd0dcf2 100644
--- a/sagenb/flask_version/worksheet.py
+++ b/sagenb/flask_version/worksheet.py
@@ -695,6 +695,7 @@ def worksheet_jsmol_data(worksheet):
             return current_app.message(_('Invalid JSmol query: ' + query))
         cell_id = match.group('cell_id')
         filename = match.group('filename')
+        filename = filename.rsplit('?',1)[0] # appended query is only for cache busting
         filename = secure_filename(filename)   # never trust input
         filename = os.path.join(worksheet.cells_directory(), cell_id, filename)
         with open(filename, 'r') as f:
diff --git a/sagenb/notebook/cell.py b/sagenb/notebook/cell.py
index 7c322c9..d0ce526 100755
--- a/sagenb/notebook/cell.py
+++ b/sagenb/notebook/cell.py
@@ -2350,7 +2350,7 @@ class Cell(Cell_generic):
             <div id="loadJmol" style="display:none;">{id}</div>
             <div id="sage_jmol_size_{id}" style="display:none;">{size}</div>
             <div id="sage_jmol_img_{id}" style="display:none;">{image_name}.png?{timestamp}</div>
-            <div id="sage_jmol_script_{id}" style="display:none;">{filename}</div>
+            <div id="sage_jmol_script_{id}" style="display:none;">{filename}?{timestamp}</div>
             <div id="sage_jmol_server_url_{id}" style="display:none;">{callback}</div>
             <div id="sage_jmol_status_{id}" style="display:none;">notActivated</div>
         </div>

[kcrisman]

comment:129

(+1) This last patch works for me on FF/linux and is reasonably simple.

Okay, that is good. I'll try to push something along these lines tonight.

If you can take a look at the CSS change stuff and possibly merge that in, then we are quite close.

[kcrisman]

comment:130

I ended up making a mess of the commit history but merged both of these things in this morning. So shortly we should merge them to sagenb, and then try to make a package for this ticket - assuming #16911 gets in first, which it definitely should.

[gutow]

comment:131

OK, I'm now synced with the new_jmol repository and everything looks OK. Are we at the positive review stage? I think I checked carefully that all of Karl-Dieter's css changes went into both the scss and the css.

[kcrisman]

comment:132

Yeah, I can't think of anything else. All your browsers work nicely?

In that case, I will merge that WHOLE pull request, with all its ugly extra commits, and then ask ppurka if we can get a new sagenb package. (I haven't really checked into how to do that yet.) It would wait until after #16911 but then hopefully we can get a nice good period of testing in with whatever beta it gets merged into.

Thanks in advance for all your very good work on this; to ask a chemist to write tons of javascript and html for mathematics is a bit above the call of duty :)

[kcrisman]

comment:133

Needs a package of course, so 'needs work'.

[kcrisman]

Branch: u/kcrisman/jsmolsagenb

[kcrisman]

New commits:

c5354ab

Upgrade sagenb to version 0.11.0

[kcrisman]

Commit: c5354ab

[kcrisman]

comment:135

Ready to try out!

[kcrisman]

Description changed:

--- 
+++ 
@@ -10,10 +10,11 @@
 Will require a new sagenb release: [INSERT HERE]

 How to test:
-1. Switch Sage to #17020
+1. Update Sage with #17020
 2. Download the repackaged Jmol from the link provide in #17020 and save the package in the "upstream" directory of your sage install.
 3. run `sage -f jmol` to install the new jmol.
-4. Clone the `new_jmol` branch from the PR to anywhere convenient on your system: `git clone https://github.com/vbraun/sagenb.git -b new_jmol`
-5. From within the sagenb repository run `sage -python setup.py install`
-
-For testing, #17170 is also advised, since without it display in the notebook does not work properly.
+4. Update Sage with the branch here
+5. Put [the new upstream package](http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.0.tar) into `SAGE_ROOT/upstream`
+6. Do `sage -f sagenb-0.11.0`
+7. Test!
+8. For testing, #17170 is also advised, since without it display in the notebook does not work properly.

[vbraun]

Changed branch from u/kcrisman/jsmolsagenb to u/vbraun/jsmolsagenb

[vbraun]

comment:137

Fixed the obvious conflict with #16911

---

New commits:

2d3d3e9	update sagenb developer guide with new dev instructions
fe9630c	update sagenb version and checksum
b84d502	Update checksums for sagenb 0.10.8.3
a4c94cd	Merge #16911

[vbraun]

Changed commit from c5354ab to a4c94cd

[kcrisman]

Description changed:

--- 
+++ 
@@ -13,8 +13,10 @@
 1. Update Sage with #17020
 2. Download the repackaged Jmol from the link provide in #17020 and save the package in the "upstream" directory of your sage install.
 3. run `sage -f jmol` to install the new jmol.
-4. Update Sage with the branch here
-5. Put [the new upstream package](http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.0.tar) into `SAGE_ROOT/upstream`
-6. Do `sage -f sagenb-0.11.0`
-7. Test!
-8. For testing, #17170 is also advised, since without it display in the notebook does not work properly.
+4. If you don't have it yet, update Sage with #16911 **or** just use this branch that Volker has merged with that one.
+5. Same with #16396 if you want to build doc correctly.
+6. Update Sage with the branch here
+7. Put [the new upstream package](http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.0.tar) into `SAGE_ROOT/upstream`
+8. Do `sage -f sagenb-0.11.0`
+9. Test!
+10. For testing, #17170 is also advised, since without it display in the notebook does not work properly.