Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#17020 closed enhancement (fixed)

Update jmol to the latest version

Reported by: vbraun Owned by:
Priority: major Milestone: sage-6.4
Component: graphics Keywords:
Cc: gutow, fbissey, strogdon, kcrisman 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 7 years ago by vbraun

  • Branch set to u/vbraun/update_jmol_to_the_latest_version

comment:2 Changed 7 years ago by vbraun

  • Commit set to 69c05983e903136be5036be62d84f99fc468b95b
  • Component changed from PLEASE CHANGE to graphics
  • Description modified (diff)
  • Status changed from new to needs_review
  • Type changed from PLEASE CHANGE to enhancement

New commits:

69c0598Update jmol to 14.2.4_2014.08.03

comment:3 Changed 7 years ago by vbraun

  • Cc gutow added

comment:4 Changed 7 years ago by chapoton

see also #16003

comment:5 Changed 7 years ago by git

  • Commit changed from 69c05983e903136be5036be62d84f99fc468b95b to 5c765f0c28f135134f5f09392076fa219b8e70c0

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 7 years ago by vbraun

  • Authors set to Jonathan Gutow, Volker Braun

comment:7 Changed 7 years ago by vbraun

  • Description modified (diff)

comment:8 Changed 7 years ago by vbraun

Everything except the last commit is imported from #16004

comment:9 Changed 7 years ago by fbissey

  • Cc fbissey added

comment:10 Changed 7 years ago by 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 7 years ago by strogdon

  • Cc strogdon added

comment:12 Changed 7 years ago by 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 7 years ago by kcrisman

  • Cc kcrisman added

comment:14 Changed 7 years ago by gutow

  • Branch changed from u/vbraun/update_jmol_to_the_latest_version to u/gutow/ticket/17020
  • Created changed from 09/20/14 23:14:37 to 09/20/14 23:14:37
  • Modified changed from 10/07/14 14:07:54 to 10/07/14 14:07:54

comment:15 Changed 7 years ago by 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 7 years ago by 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 7 years ago by kcrisman

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 7 years ago by gutow

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

comment:19 Changed 7 years ago by kcrisman

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 7 years ago by git

  • Commit changed from 5c765f0c28f135134f5f09392076fa219b8e70c0 to 62f74ad915f5d729f87515d6daa2d521d8f021c5

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

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

comment:21 Changed 7 years ago by kcrisman

The fix on the version works great now, thanks.

comment:22 follow-up: Changed 7 years ago by 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...

Last edited 7 years ago by kcrisman (previous) (diff)

comment:23 in reply to: ↑ 22 Changed 7 years ago by 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 7 years ago by kcrisman

Not too worried about it, anyway.

comment:25 Changed 7 years ago by kcrisman

  • Reviewers set to 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 7 years ago by 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 7 years ago by fbissey

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 7 years ago by gutow

  • Status changed from needs_review to positive_review

comment:29 Changed 7 years ago by vbraun

  • Branch changed from u/gutow/ticket/17020 to 62f74ad915f5d729f87515d6daa2d521d8f021c5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:30 Changed 7 years ago by kcrisman

  • Commit 62f74ad915f5d729f87515d6daa2d521d8f021c5 deleted
  • Dependencies set to #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 follow-up: Changed 7 years ago by vbraun

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 7 years ago by vbraun

  • Branch changed from 62f74ad915f5d729f87515d6daa2d521d8f021c5 to u/gutow/ticket/17020
  • Commit set to 62f74ad915f5d729f87515d6daa2d521d8f021c5
  • Resolution fixed deleted
  • Status changed from closed to new

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 7 years ago by vbraun

  • Dependencies changed from #17010 to #16004

comment:34 in reply to: ↑ 31 Changed 7 years ago by kcrisman

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 7 years ago by kcrisman

  • Status changed from new to needs_review

comment:36 Changed 7 years ago by kcrisman

  • Status changed from needs_review to positive_review

comment:37 Changed 7 years ago by vbraun

  • Branch changed from u/gutow/ticket/17020 to u/vbraun/ticket/17020

comment:38 Changed 7 years ago by vbraun

  • Branch changed from u/vbraun/ticket/17020 to 62f74ad915f5d729f87515d6daa2d521d8f021c5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:39 Changed 7 years ago by jdemeyer

  • Commit 62f74ad915f5d729f87515d6daa2d521d8f021c5 deleted

Please see #17279 for a blocker follow-up.

Note: See TracTickets for help on using tickets.