Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#17020 closed enhancement (fixed)

Update jmol to the latest version

Reported by: Volker Braun Owned by:
Priority: major Milestone: sage-6.4
Component: graphics Keywords:
Cc: Jonathan Gutow, François Bissey, Steven Trogdon, Karl-Dieter Crisman Merged in:
Authors: Jonathan Gutow, Volker Braun Reviewers: Karl-Dieter Crisman, Jonathan Gutow, Volker Braun
Report Upstream: N/A Work issues:
Branch: 62f74ad (Commits, GitHub, GitLab) Commit:
Dependencies: #16004 Stopgaps:

Status badges

Change History (39)

comment:1 Changed 8 years ago by Volker Braun

Branch: u/vbraun/update_jmol_to_the_latest_version

comment:2 Changed 8 years ago by Volker Braun

Commit: 69c05983e903136be5036be62d84f99fc468b95b
Component: PLEASE CHANGEgraphics
Description: modified (diff)
Status: newneeds_review
Type: PLEASE CHANGEenhancement

New commits:

69c0598Update jmol to 14.2.4_2014.08.03

comment:3 Changed 8 years ago by Volker Braun

Cc: Jonathan Gutow added

comment:4 Changed 8 years ago by Frédéric Chapoton

see also #16003

comment:5 Changed 8 years ago by git

Commit: 69c05983e903136be5036be62d84f99fc468b95b5c765f0c28f135134f5f09392076fa219b8e70c0

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e417d3bJmol/JSmol to 14.0.11
049657ftypo fixes to Jmol/SPKG.txt
f9e55eeupdate Jmol/JSmol popup menu to include file actions
a89ac4bupdate Jmol/JSmol to version 14.0.13
f74866eRebased on 6.3.rc1
8d0bd28Merge branch 'public/ticket/16003' of trac.sagemath.org:sage into jsmol
0071f18Remove unnecessary testjava.sh test
f8ba6aaupdate version of Jmol to Jonathan's last spkg
5c765f0Update to jmol 14.2.4_2014.08.03

comment:6 Changed 8 years ago by Volker Braun

Authors: Jonathan Gutow, Volker Braun

comment:7 Changed 8 years ago by Volker Braun

Description: modified (diff)

comment:8 Changed 8 years ago by Volker Braun

Everything except the last commit is imported from #16004

comment:9 Changed 8 years ago by François Bissey

Cc: François Bissey added

comment:10 Changed 8 years ago by Jonathan Gutow

Volker, Thank you for the work making this mesh with the flask server better. I have been bogged down with work and getting an upstream fix so that Java applets can act just like the javascript version and have a static image put up first instead of immediately launching the applet. I have pushed that to Bob Hanson for final approval and folding into the JSmol javascript libraries. Hopefully, that will be in JSmol/Jmol soon. It should have no effect on this, but I will start testing what you've put together against that now.

Jonathan

comment:11 Changed 8 years ago by Steven Trogdon

Cc: Steven Trogdon added

comment:12 Changed 8 years ago by Jonathan Gutow

It looks OK except for a some things that have been lost:

  1. The use Java clickbox is gone. People have requested that that be an option and I have pushed a patch upstream that should give exactly the same static image first behavior from the java applet.
  2. The specialized right click menu for Sage has been lost (or did I not push that to my repository).
  3. Do we want the message reminding people that they can click to make the image live and right-click for the menu?

I've also got grab to resize working on my system. I will try to merge it with this.

comment:13 Changed 8 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:14 Changed 8 years ago by Jonathan Gutow

Branch: u/vbraun/update_jmol_to_the_latest_versionu/gutow/ticket/17020
Created: Sep 20, 2014, 11:14:37 PMSep 20, 2014, 11:14:37 PM
Modified: Oct 7, 2014, 2:07:54 PMOct 7, 2014, 2:07:54 PM

comment:15 Changed 8 years ago by Jonathan Gutow

This commit fixes the special menu and allows jquery drag resizing to work on the JSmol/Jmol applet. Still need to decide how to handle user requesting to use the Jmol java applet.

comment:16 Changed 8 years ago by Jonathan Gutow

I'm confused. The commit that sage made does not seem to include the changes to the jmol_lib.js file from sagenb...Am I on the wrong ticket here? Jonathan

comment:17 Changed 8 years ago by Karl-Dieter Crisman

I don't think so, but Sage won't do any commits to sagenb, that will have to be upstream, I guess.

Also, I'm back and should have some time this week to review the various tickets involved in this, so if that can get figured out that would be awesome!

comment:18 Changed 8 years ago by Jonathan Gutow

The fixes to sagenb need to be on ticket #16004. I uploaded them as patches.

comment:19 Changed 8 years ago by Karl-Dieter Crisman

So what needs to be done for review here? This is only about upgrading Jmol (which of course needs #16004 to work properly in the notebook). Jonathan, are you happy with this update? Since you seem to have approved of the changes to the Sage library, that is a good sign!

comment:20 Changed 8 years ago by git

Commit: 5c765f0c28f135134f5f09392076fa219b8e70c062f74ad915f5d729f87515d6daa2d521d8f021c5

Branch pushed to git repo; I updated commit sha1. New commits:

62f74adUpdated Jmol/JSmol menu to get and show actual version.

comment:21 Changed 8 years ago by Karl-Dieter Crisman

The fix on the version works great now, thanks.

comment:22 Changed 8 years ago by Karl-Dieter Crisman

I did get a strange message about invalid request when I tried to save a jsmol as jmol+png but I don't know that very many people are using that...

Version 0, edited 8 years ago by Karl-Dieter Crisman (next)

comment:23 in reply to:  22 Changed 8 years ago by Jonathan Gutow

Replying to kcrisman:

I did get a strange message about invalid request when I tried to save a jsmol as jmol+png (from right-click in the notebook) but I don't know that very many people are using that...

Is this with the old Safari? I cannot reproduce it. Works fine in FF and Chromium.

comment:24 Changed 8 years ago by Karl-Dieter Crisman

Not too worried about it, anyway.

comment:25 Changed 8 years ago by Karl-Dieter Crisman

Reviewers: Karl-Dieter Crisman, Jonathan Gutow, Volker Braun

In some sense Volker already reviewed the stuff that had been at #16004, Jonathan reviewed the stuff that is new here... kiwifb, Steve, any desire to input?

comment:26 Changed 8 years ago by Jonathan Gutow

I approve. If nobody else objects, I give this a positive review. I'll wait until this evening to give people a chance.

comment:27 Changed 8 years ago by François Bissey

No further useful input from me. In useless input I must say jmol is a difficult thing to package for Gentoo and each upgrade has been more difficult than the last. This is not distro friendly.

comment:28 Changed 8 years ago by Jonathan Gutow

Status: needs_reviewpositive_review

comment:29 Changed 8 years ago by Volker Braun

Branch: u/gutow/ticket/1702062f74ad915f5d729f87515d6daa2d521d8f021c5
Resolution: fixed
Status: positive_reviewclosed

comment:30 Changed 8 years ago by Karl-Dieter Crisman

Commit: 62f74ad915f5d729f87515d6daa2d521d8f021c5
Dependencies: #17010

Not sure why this didn't get in the description, but this has a dependency (both directions) with #16004, so I don't know if you can just merge it. This is mentioned obliquely in various comments, but that wasn't explicit enough, sorry about this.

comment:31 Changed 8 years ago by Volker Braun

Guys, I can't possibly read all the text when merging. If you don't want stuff merged then you need to make it dependent on the unfinished ticket.

comment:32 Changed 8 years ago by Volker Braun

Branch: 62f74ad915f5d729f87515d6daa2d521d8f021c5u/gutow/ticket/17020
Commit: 62f74ad915f5d729f87515d6daa2d521d8f021c5
Resolution: fixed
Status: closednew

New commits:

e417d3bJmol/JSmol to 14.0.11
049657ftypo fixes to Jmol/SPKG.txt
f9e55eeupdate Jmol/JSmol popup menu to include file actions
a89ac4bupdate Jmol/JSmol to version 14.0.13
f74866eRebased on 6.3.rc1
8d0bd28Merge branch 'public/ticket/16003' of trac.sagemath.org:sage into jsmol
0071f18Remove unnecessary testjava.sh test
f8ba6aaupdate version of Jmol to Jonathan's last spkg
5c765f0Update to jmol 14.2.4_2014.08.03
62f74adUpdated Jmol/JSmol menu to get and show actual version.

comment:33 Changed 8 years ago by Volker Braun

Dependencies: #17010#16004

comment:34 in reply to:  31 Changed 8 years ago by Karl-Dieter Crisman

Guys, I can't possibly read all the text when merging. If you don't want stuff merged then you need to make it dependent on the unfinished ticket.

I know that, which is why I said sorry.

PS To be fair, YOU are the one who made the branch on this ticket, though! At that time you didn't make the dependency on #16004 explicit either, and in fact, I think you confirmed that you tried this one without the other on some ticket or other! Just sayin'.

PPS thanks for unmerging. I just haven't had the chance to make the package yet for #16004. You even beat me to putting in the dependency just now :-)

comment:35 Changed 8 years ago by Karl-Dieter Crisman

Status: newneeds_review

comment:36 Changed 8 years ago by Karl-Dieter Crisman

Status: needs_reviewpositive_review

comment:37 Changed 8 years ago by Volker Braun

Branch: u/gutow/ticket/17020u/vbraun/ticket/17020

comment:38 Changed 8 years ago by Volker Braun

Branch: u/vbraun/ticket/1702062f74ad915f5d729f87515d6daa2d521d8f021c5
Resolution: fixed
Status: positive_reviewclosed

comment:39 Changed 8 years ago by Jeroen Demeyer

Commit: 62f74ad915f5d729f87515d6daa2d521d8f021c5

Please see #17279 for a blocker follow-up.

Note: See TracTickets for help on using tickets.