New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

defaultTracks in config as a comma list not parsed in 1.12.3 #892

Closed
rdhayes opened this Issue May 8, 2017 · 3 comments

Comments

Projects
None yet
5 participants
@rdhayes
Contributor

rdhayes commented May 8, 2017

Hi,

We have defaultTracks in trackList.json as a comma separated list:
"defaultTracks" : "Transcripts,PASA_assembly,Blastx_protein,Blatx_Monocots",

With JBrowse v1.12.3, and a newly emptied browser and cookie store, when I attempt to load a genome with just the data parameter (which should trigger those defaults), I instead only receive the DNA track.

The new logic in Browser.js is the culprit from lines 190 to 202.

The first time through with a new page load and no cookie set triggered the

 else if (thisB.config.defaultTracks) {

block at line 192 is entered, but the type check

 if (!thisB.config.defaultTracks instanceof Array) {

on line 197 fails and does not populate tracksToShow as intended. This leaves only the unconfigured default of "DNA"

This looks like it initially was introduced in commit b415253 to address GMOD/Apollo#1498

I'm not familiar with what would alter the config value to be an array rather than string, but this slightly different syntax restores correct use of defaultTracks in regular JBrowse 1.12.3 for me.

  if (thisB.config.defaultTracks instanceof Array) {
        tracksToShow = tracksToShow.concat(thisB.config.defaultTracks);
  }
  else {
        tracksToShow = tracksToShow.concat(thisB.config.defaultTracks.split(","));
  }

I'm happy to submit a pull request for this minor change if that looks good.

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented May 10, 2017

Yep looks like the intended commit was probably to use if (!(thisB.config.defaultTracks instanceof Array)) but the alternative fix you posted seems good too

@rdhayes

This comment has been minimized.

Contributor

rdhayes commented May 10, 2017

Looking at this again, my fix always adds defaultTracks to the trackToShow list, but Eric's original intention seems to only concat() if not an array. I think my proposal might end up building an array like [ "track1", "track2, track3" ] for Apollo default tracksToShow, which would be obviously wrong.

@cmdcolin 's extra enclosing () fixes this for me in a dev build of v1.12.3 and is the proper patch.

@erasche what do you think?

Thanks!

@erasche

This comment has been minimized.

Contributor

erasche commented May 10, 2017

argh, so sorry I introduced such a bug. yes, that looks like what I intended, that it should be converted only if an array.

rdhayes pushed a commit that referenced this issue May 31, 2017

JGI Compgen Web User

@rbuels rbuels added this to the 1.12.4 milestone Jan 30, 2018

@rbuels rbuels added the bug label Jan 30, 2018

@rbuels rbuels closed this in #896 Jan 30, 2018

@wafflebot wafflebot bot removed the in progress label Jan 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment