1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-24 22:40:55 +01:00
Commit graph

23 commits

Author SHA1 Message Date
epriestley
02315c4c48 Fix double-close on dialogs leading to Javascript console error
Summary:
Ref T13302. The "Close/Cancel" button is currently running two copies of the "dismiss dialog" code, since it's techncally a link with a valid HREF attribute.

An alternate formulation of this is perhaps `if (JX.Stratcom.pass()) { return; }` ("let other handlers react to this event; if something kills it, stop processing"), but `pass()` is inherently someone spooky/fragile so try to get away without it.

Test Plan: Opened the Javascript console, clicked "Edit Task" on a workboard, clicked "Close" on the dialog. Before: event was double-handled leading to a JS error in the console. After: dialog closes uneventfully.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13302

Differential Revision: https://secure.phabricator.com/D20640
2019-07-03 12:38:49 -07:00
epriestley
d6dc5d8e68 Fix the "x" link in tokenizer tokens incorrectly closing dialogs
Summary:
See <https://discourse.phabricator-community.org/t/removing-tags-or-subscribers-from-a-maniphest-modal-causes-the-modal-to-close/2874>.

My Javascript is rusty: `'' + null == 'null'`. Same for `undefined`. Use an explicit typecheck instead.

Test Plan: Clicked the "x" in a tokenizer token in a dialog, saw the token removed instead of the dialog closed.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20623
2019-06-28 12:38:51 -07:00
epriestley
9f44ee3933 Use "link.getAttribute('href')", not "link.href", to bypass dark browser magic
Summary:
Ref T13302. In at least some browsers (including Safari and Chrome), when you write this:

```
<a href="#">...</a>
```

...and then access `<that node>.href`, you get `http://local-domain-whatever.com/path/to/current/page#` back.

This is wonderful, but not what we want. Access the raw attribute value instead, which is `#` in all browsers.

Test Plan:
  - In Safari, Chrome, and Firefox:
  - Clicked "Edit Subtasks" from a task.
  - Clicked "Select" buttons to select several tasks.
  - Before: Clicking these button incorrectly closed the dialog (because of D20573).
  - After: Clicking these buttons now selects tasks without closing the dialog.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13302

Differential Revision: https://secure.phabricator.com/D20590
2019-06-19 10:44:42 -07:00
epriestley
dcf3ca8e04 When a user clicks a navigation link in a dialog, close the dialog
Summary:
Ref T13302. Currently, if you enable Quicksand (by clicking "Persistent Chat"), open a dialog with links in it (like "Create Subtask" with multiple available subtypes), and then follow a navigation link, the page content reloads behind the dialog but the dialog stays in the foreground.

Fix this by closing dialogs when users click navigation links inside them.

Test Plan:
With Quicksand enabled and disabled, clicked a subtask type in the "Create Subtask" dialog.

  - Before, Quicksand Disabled: Dialog stays on screen, then navigation occurs.
  - After, Quicksand Disabled: Dialog vanishes, then navigation occurs.
  - Before, Quicksand Enabled: Dialog stays on screen, navigation occurs behind it.
  - After, Quicksand Enabled: Dialog vanishes, then navigation occurs.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13302

Differential Revision: https://secure.phabricator.com/D20573
2019-06-18 15:16:32 -07:00
epriestley
e010aaca43 accidentally a word
Summary: Sometimes I dream I am a small turtle.

Test Plan: squeak squeak

Differential Revision: https://secure.phabricator.com/D19248
2018-03-22 13:43:10 -07:00
epriestley
c8583b016d When workflow dialog buttons are clicked, disable the button
Summary:
Depends on D19245. Fixes T11145. Ref T13108. See PHI488. Disable workflow buttons when they're clicked to prevent accidental client-side double submission.

This might have some weird side effects but we should normally never need to re-use a workflow dialog form so it's not immediately obvious that this can break anything.

Test Plan:
  - Added `sleep(1)` to the Mute controller and the Maniphest task controller.
  - Added `phlog(...)` to the Mute controller.
  - Opened the mute dialog, mashed the button a thousand times.
    - Before: Saw a bunch of logs.
    - After: Button immediately disables, saw only one log.

Maniphest Tasks: T13108, T11145

Differential Revision: https://secure.phabricator.com/D19246
2018-03-21 11:58:13 -07:00
epriestley
9e278a89ba If a Workflow form receives a redirect response, don't re-enable the submit buttons
Summary:
See PHI488. Ref T13108. Currently, there is a narrow window between when the response returns and when the browser actually follows the redirect where the form is live and you can click the button again.

This is relativey easy if Phabricator is running //too fast// since the button may be disabled only momentarily. This seems to be easier in Firefox/Chrome than Safari.

Test Plan:
  - In Firefox and Chrome, spam-clicked a comment submit button.
    - Before: could sometimes get a double-submit.
    - After: couldn't get a double-submit.
    - This could probably be reproduced more reliabily by adding a `sleep(1)` to whatever we're redirecting //to//.
  - Submitted an empty comment, got a dialog plus a still-enabled form (so this doesn't break the non-redirect case).

Maniphest Tasks: T13108

Differential Revision: https://secure.phabricator.com/D19245
2018-03-21 11:56:21 -07:00
epriestley
ab579f2511 Never generate file download forms which point to the CDN domain, tighten "form-action" CSP
Summary:
Depends on D19155. Ref T13094. Ref T4340.

We can't currently implement a strict `form-action 'self'` content security policy because some file downloads rely on a `<form />` which sometimes POSTs to the CDN domain.

Broadly, stop generating these forms. We just redirect instead, and show an interstitial confirm dialog if no CDN domain is configured. This makes the UX for installs with no CDN domain a little worse and the UX for everyone else better.

Then, implement the stricter Content-Security-Policy.

This also removes extra confirm dialogs for downloading Harbormaster build logs and data exports.

Test Plan:
  - Went through the plain data export, data export with bulk jobs, ssh key generation, calendar ICS download, Diffusion data, Paste data, Harbormaster log data, and normal file data download workflows with a CDN domain.
  - Went through all those workflows again without a CDN domain.
  - Grepped for affected symbols (`getCDNURI()`, `getDownloadURI()`).
  - Added an evil form to a page, tried to submit it, was rejected.
  - Went through the ReCaptcha and Stripe flows again to see if they're submitting any forms.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13094, T4340

Differential Revision: https://secure.phabricator.com/D19156
2018-02-28 17:20:12 -08:00
epriestley
3198aa1659 Allow dialogs to opt in to being resizable
Summary: Ref T11034. Ref T4788. This allows you to resize the typeahead browse dialog if you want. I plan to let you resize the object selector dialog in the future.

Test Plan: {F1695433}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4788, T11034

Differential Revision: https://secure.phabricator.com/D16156
2016-06-20 19:13:58 -07:00
epriestley
f2c36a934e Provide an <input type="file"> control in Remarkup for mobile and users with esoteric windowing systems
Summary:
Ref T5187. This definitely feels a bit flimsy and I'm going to hold it until I cut the release since it changes a couple of things about Workflow in general, but it seems to work OK and most of it is fine.

The intent is described in T5187#176236.

In practice, most of that works like I describe, then the `phui-file-upload` behavior gets some weird glue to figure out if the input is part of the form. Not the most elegant system, but I think it'll hold until we come up with many reasons to write a lot more Javascript.

Test Plan:
Used both drag-and-drop and the upload dialog to upload files in Safari, Firefox and Chrome.

{F1653716}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5187

Differential Revision: https://secure.phabricator.com/D15953
2016-05-20 16:24:22 -07:00
epriestley
c286d2b441 Don't try to center dialogs horizontally
Summary:
Fixes T10302. I think we had fixed-width dialog containers in the past (?) but they all handle their own centering now.

This was causing them to be slightly off-center as a result, and creating the 7px issue in T10302.

Test Plan:
  - Viewed a wide dialog (task edit).
  - Viewed a narrow dialog (notification dismissal confirmation).
  - Viewed dialogs on wide/narrow screens.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10302

Differential Revision: https://secure.phabricator.com/D15529
2016-03-26 12:29:36 -07:00
epriestley
e5580d038d Improve Conpherence column textarea JS behaviors
Summary:
  - Don't show a loading state on the whole column while sending chat. We could show some kind of minor loading state, but standard JX.Busy stuff will kick in after a couple seconds anyway.
  - Blank the textarea immediately on submit so you can start typing more text.
  - Don't disable the form while submiting; disabling it prevents you from typing more text.
  - Hide the placeholder while the textarea is focused. If we don't do this, the placeholder reappearing after submitting text feels weird to me.

Test Plan:
  - Sent a lot of text.
  - Real fast.
  - Focused and unfocused the area.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12086
2015-03-14 12:00:17 -07:00
epriestley
31c64b6772 Fix dialog z-index issue
Summary:
Fixes T7081. History here:

  - JX.Scrollbar made the page scroll weird when a dialog came up because it was half-frame and half-document.
  - I made it fully frame-level.
  - But this wasn't really right; a better fix is to make it fully document-level.

Test Plan:
  - Weird scroll on opening dialog is still fixed.
  - iOS Safari no longer puts the mask over the dialog.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7081

Differential Revision: https://secure.phabricator.com/D11559
2015-01-29 07:10:14 -08:00
epriestley
41dcd7420b Fix inline reticles and dialogs for new scrollbars
Summary:
Fixes T7054. Fixes T7049.

  - Stop scrolling Differential reticles when the page scrolls.
  - Make dialogs aware of multi-panel UI.

Test Plan:
  - Dialogs pop up in the right place.
  - Inline + scroll now longer leaves the inline in a fixed position.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7049, T7054

Differential Revision: https://secure.phabricator.com/D11523
2015-01-27 07:11:20 -08:00
epriestley
ea67a8ab8e Make scrolling happen relative to the main content frame
Summary: Fixes T7033. When we've reframed the main page content we need to scroll relative to the containing frame, not relative to the window.

Test Plan:
In Safari, Chrome and Firefox, used j/k/J/K keys to navigate diff content.

Tried some other scroll-based beahviors, like jump-to-anchors.

(It looks like the highlighting reticle got slightly derped a while ago, but it's still functional, so I didn't mess with it.)

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7033

Differential Revision: https://secure.phabricator.com/D11490
2015-01-25 08:42:40 -08:00
Joshua Spence
8756d82cf6 Remove @group annotations
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.

Test Plan: Eye-ball it.

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9859
2014-07-10 08:12:48 +10:00
epriestley
2b4c551b0e Provide a global router for Ajax requests
Summary:
Fixes T430. Fixes T4834. Obsoletes D7641. Currently, we do some things less-well than we could:

  - We just let the browser queue and prioritize requests, so if you load a revision with 50 changes and then click "Award Token", the action blocks until the changes load in most/all browsers. It would be better to prioritize this action and queue it immediately.
  - Similarly, changes tend to load in order, even if the user has clicked to a specific file. When the user expresses a preference for a specific file, we should prioritize it.
  - We show a spinning GIF when waiting on requests. This is appropriate for some types of reuqests, but distracting for others.

To fix this:

  - Queue all (or, at least, most) requests into a new queue in JX.Router.
  - JX.Router handles prioritizing the requests. Principally:
    - You can submit a request with a specific priority (500 = general content loading, 1000 = default, 2000 = explicit user action) and JX.Router will get the higher stuff fired off sooner.
    - You can name requests and then adjust their prorities later, if the user expresses an interest in specific results.
  - Only use the spinner gif for "workflow" requests, which is bascially when the user clicked something and we're waiting on the server. I think it's useful and not-annoying in this case.
  - Don't show any status for draft requests.
  - For content requests, show a subtle hipster-style top loading bar.

Test Plan:
  - Viewed a diff with 93 changes, and clicked award token.
    - Prior to this patch, the action took many many seconds to resolve.
    - After this patch, it resolves quickly.
  - Viewed a diff with 93 changes and saw a pleasant subtle hipster-style loading bar.
  - Viewed a diff with 93 changes and typed some draft text. Previews populated fairly quickly and there was no spinner.
  - Viewed a diff with 93 changes and clicked something with workflow, saw a spinner after a moment.
  - Viewed a diff with 93 changes and clicked a file in the table of contents near the end of the list.
    - Prior to this patch, it took a long time to show up.
    - After this patch, it loads directly.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T430, T4834

Differential Revision: https://secure.phabricator.com/D8979
2014-05-05 10:57:42 -07:00
Bob Trahan
c995e93bc1 Transactions - make ctrl + enter submit ajax-style
Summary: We were correctly invoked a didSyntheticSubmit event on the form, but nothing was listening to it. Re-jigger the workflow submit code a tad so an onsyntheticsubmit event handler can be written to fill this gap. Fixes T4669.

Test Plan: edited comments, submitting both via clicking the button and apple + enter LIKE A BOSS

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4669

Differential Revision: https://secure.phabricator.com/D8961
2014-05-02 18:01:16 -07:00
epriestley
44fc671b3f Add a "Generate Keypair" option on the SSH Keys panel
Summary: Ref T4587. Add an option to automatically generate a keypair, associate the public key, and save the private key.

Test Plan: Generated some keypairs. Hit error conditions, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: aran, epriestley

Maniphest Tasks: T4587

Differential Revision: https://secure.phabricator.com/D8513
2014-03-12 18:17:11 -07:00
epriestley
3f5a7b31c8 Add a synthetic DOM event to JX.Workflow for form submission
Summary: Currently, when a dialog is submitted the Workflow itself emits an event but no DOM event is emitted. The workflow event is fine for handlers which only use JS, but there's currently no way for a handler to act more like a normal form handler. This event gives normal form handlers a way to capture Workflow submits and muck around with form contents, etc.

Test Plan: In a future diff, edited policies via a Workflow dialog.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7295
2013-10-14 11:58:38 -07:00
epriestley
282b3f7988 Allow Maniphest tasks to be edited from the list view
Summary:
Fixes T1945. Ref T2947. At various times, installs (Disqus, Dropbox, etc.) have asked for a way to edit tasks more quickly. Provide edit-from-lists.

{F44700}

{F44701}

The one rough edge on this is that if you change the task priority we update it inline but don't move it. It's probably infeasible to actually move it, but maybe we could give it some sort of visual style to indicate that it's dirty.

Test Plan: Edited tasks normally and via this action thing.

Reviewers: chad, btrahan

Reviewed By: chad

CC: tido, deuresti, ahoffer, aran

Maniphest Tasks: T1945, T2947

Differential Revision: https://secure.phabricator.com/D6086
2013-05-30 18:55:25 -07:00
epriestley
0569218201 Use JsShrink if jsxmin is not available
Summary: If `jsxmin` is not available, use a pure PHP implementation instead (JsShrink).

Test Plan:
  - Ran `arc lint --lintall` on all JS and fixed every relevant warning.
  - Forced minification on and browsed around the site using JS behaviors. Didn't hit anything problematic.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5670
2013-05-18 17:04:22 -07:00
epriestley
95f57756aa Move all external JS into webroot/rsrc/externals
Summary:
Currently, Celerity map rebuilds on Windows don't put Stripe or Raphael into the map. Move them into `webroot/rsrc/externals/` so they get picked up.

At some point we should maybe let the mapper load resources from mulitple locations, but this is more straightforward for now.

See https://github.com/facebook/phabricator/issues/294

Test Plan: Rebuilt map, verified Burnup Rate + Stripe work.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D5661
2013-04-11 10:06:05 -07:00
Renamed from webroot/rsrc/js/javelin/lib/Workflow.js (Browse further)