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

Download fixes #5125

Merged
merged 7 commits into from Dec 19, 2017

Conversation

Projects
None yet
4 participants
@dannon
Member

dannon commented Dec 4, 2017

Prevents 'download' directive from behaving badly on firefox. (even a right click attempts to download the file, so you can't copy the url).

Adds the download URL to multifile download icons, allowing copy/paste. We preventDefault, so the menu still works as expected, and the void(0) is unnecesssary.

@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Dec 14, 2017

I confirm this fixes the issue, thanks @dannon, I'll just rebuild the client and merge.

@dannon

This comment has been minimized.

Member

dannon commented Dec 14, 2017

Thanks @mvdbeek; I pushed a rebuild. I saw a failing tool form selenium test, but I don't remember seeing it before. Trying again locally to make sure it isn't related to this code.

Followup: Selenium test mentioned https://jenkins.galaxyproject.org/job/docker-selenium/665/ passes fine locally (and may on rerun, here -- seems a transient failure).

@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Dec 14, 2017

Hmm,

"value": ["/", "t", "m", "p", "/", "6", "3", "6", "0", "9", "9", "b", "e", "1", "f", "c", "9", "0", "8", "9", "d", "3", "b", "a", "f", "a", "9", "d", "a", "6", "c", "9", "0", "3", "7", "c", "8", "/", "u", "p", "l", "o", "a", "d", "8", "9", "9", "1", "0", "3", "4", "4", "5", "7", "2", "9", "2", "3", "4", "1", "4", "1", "8", "f", "i", "l", "e", "/", "1", ".", "f", "a", "s", "t", "a"]}

that looks weird, like we're iterating on a string while we should be iterating on something else ... .
There were supposed to be screenshots for failing tests, right ?

@dannon

This comment has been minimized.

Member

dannon commented Dec 14, 2017

Yeah, in https://jenkins.galaxyproject.org/job/docker-selenium/665/artifact/

(it looks like maybe from that screenshot it just didn't wait long enough)

@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Dec 14, 2017

Hmm, it failed again, and the history clearly has a hid. I wonder if it's checking the wrong history?
Is https://github.com/galaxyproject/galaxy/blob/dev/test/galaxy_selenium/navigates_galaxy.py#L207 really guaranteed to return the current history ?

@dannon

This comment has been minimized.

Member

dannon commented Dec 14, 2017

I'd think we would have noticed that before now, but I guess it's possible?

@dannon

This comment has been minimized.

Member

dannon commented Dec 14, 2017

xref #5211
The problem isn't this PR.

@dannon

This comment has been minimized.

Member

dannon commented Dec 19, 2017

Rebuilt again.

@bgruening

This comment has been minimized.

Member

bgruening commented Dec 19, 2017

works for me as well...

@bgruening bgruening merged commit be44b9b into galaxyproject:dev Dec 19, 2017

6 checks passed

api test Build finished. 336 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 165 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 60 tests run, 0 skipped, 0 failed.
Details
selenium test Build finished. 117 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@dannon

This comment has been minimized.

Member

dannon commented Dec 19, 2017

Thanks @bgruening!

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