Bug #5198

[Workbench] Middle clicking on Run this pipeline brings fiddlesticks

Added by Bryan Cosca over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Start date:
02/12/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

middleclickrun.gif (134 KB) middleclickrun.gif Bryan Cosca, 02/12/2015 03:34 PM

Subtasks

Task #5507: Review 5198-remote-link-ctrl-clickResolvedTom Clegg

Task #5498: Prohibit accidentally ctrl-clickable remote linksResolvedTom Clegg


Related issues

Related to Arvados - Bug #5541: [Workbench] Buttons should have consistent ctrl-click and right-click behaviorNew03/23/2015

Associated revisions

Revision bedc4bf7
Added by Tom Clegg over 4 years ago

Merge branch '5198-remote-link-ctrl-click' refs #5198

History

#1 Updated by Brett Smith over 4 years ago

  • Subject changed from Middle clicking on Run this pipeline brings fiddlesticks to [Workbench] Middle clicking on Run this pipeline brings fiddlesticks
  • Category set to Workbench
  • Target version set to Bug Triage

This is almost certainly the same basic problem as #4291. We really need to be using form buttons in any situation where we can't make sure a link goes to the right place.

#2 Updated by Tom Clegg over 4 years ago

  • Status changed from New to In Progress
  • Story points set to 0.5

#3 Updated by Tom Clegg over 4 years ago

  • Assigned To set to Tom Clegg

#4 Updated by Tom Clegg over 4 years ago

  • Target version changed from Bug Triage to 2015-04-01 sprint

#5 Updated by Radhika Chippada over 4 years ago

Branch looked good overall. Some of my observations:

  • Ctrl-click on pipeline instance “Re-run options” is working in staging already and I see the dialog. It is of course working the same way in the new branch as well
  • Right click -> "Open in new tab" working both in staging and new branch on “Re-run options”
  • Right click -> "Open in new tab" on Pipeline instance -> “Re-run with latest” button shows fiddlesticks in staging. In the branch, the right click does not show the “Open in new tab” link though. It shows drop down menu with Back, Forward, Reload and other options only. All in all, this is the one improvement I observed in this branch since Ctrl-click is already working.
  • Project -> "Run a pipeline…" button: Ctrl+click does not do anything (Ctrl-click on Move project works as before)
  • The comments in helpers/application_helper.rb:
    • I see this comment “ When using {remote:true} or {method:...}, move the target URI from …” . Would you update this comment to say non-GET methods since we are changing only when method != GET?
    • This comment: “but there's no ‘copy link address’ option in the right-click menu” … I saw this in right-click menu (in the branch) and it also worked as expected.
  • I did some manual testing around pipeline running and re-running but did not do extensive manual testing
  • I ran only the project and pipeline integration tests, unit and controller tests (which all passed) and did not run all the other workbench integration tests …
  • The diagnostics test failed for me but I did not spend much time debugging this. As long as you tested it and it is working for you, I am ok with it.

#6 Updated by Tom Clegg over 4 years ago

Radhika Chippada wrote:

Branch looked good overall. Some of my observations:

  • Ctrl-click on pipeline instance “Re-run options” is working in staging already and I see the dialog. It is of course working the same way in the new branch as well
  • Right click -> "Open in new tab" working both in staging and new branch on “Re-run options”

True, that button didn't exhibit this bug. It uses href="#" and Bootstrap data-toggle: if you ctrl-click it, you get a dialog, and if you "copy link address" and paste it somewhere, you just get the current page again. (This seems less serious than giving a Fiddlesticks, but I'd characterize it as "still weird" rather than "still working"...)

  • Right click -> "Open in new tab" on Pipeline instance -> “Re-run with latest” button shows fiddlesticks in staging. In the branch, the right click does not show the “Open in new tab” link though. It shows drop down menu with Back, Forward, Reload and other options only. All in all, this is the one improvement I observed in this branch since Ctrl-click is already working.

Right. The fix here is: when the button makes an HTTP request that isn't GET or doesn't return HTML, it doesn't make sense to offer "open in new tab" or "copy link address" -- you'd just get an error, or some unexpected behavior -- so don't offer those things. For example, there is no link address that you can copy and paste to get the "re-run with latest" action, so that button shouldn't offer to "copy link address" or "open in a new tab".

  • Project -> "Run a pipeline…" button: Ctrl+click does not do anything (Ctrl-click on Move project works as before)

Hm, the difference here seems to be that "Run a pipeline..." doesn't specify data-method. Haven't dug deeper than that.

  • The comments in helpers/application_helper.rb:
    • I see this comment “ When using {remote:true} or {method:...}, move the target URI from …” . Would you update this comment to say non-GET methods since we are changing only when method != GET?

Updated, 17efed0

  • This comment: “but there's no ‘copy link address’ option in the right-click menu” … I saw this in right-click menu (in the branch) and it also worked as expected.

Hm. Which button is still doing this? Is it a button that was affected by this bug/helper? If you copy that link address into the location bar, does it show something reasonable?

  • The diagnostics test failed for me but I did not spend much time debugging this. As long as you tested it and it is working for you, I am ok with it.

Diagnostics (and other) tests pass for me.

#7 Updated by Radhika Chippada over 4 years ago

Tom said:

This comment: “but there's no ‘copy link address’ option in the right-click menu” … I saw this in right-click menu (in the branch) and it also worked as expected.

Hm. Which button is still doing this? Is it a button that was affected by this bug/helper? If you copy that link address into the location bar, does it show something reasonable?

You are right. This is the other "Rerun with options" button. So, please ignore my comment.

Diagnostics (and other) tests pass for me

My diagnostics testing config was out of date. I updated it and it is working for me as well. It is not impacted by work in this branch.

LGTM

#8 Updated by Tom Clegg over 4 years ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF