Fixes T5188; see task for discussion.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T5188: Homepage file drag & drop locks up on Firefox when pressing ESC
- Commits
- Restricted Diffusion Commit
rP6a66d19a2d0f: Global upload - fix for Firefox
made error condition occur with Firefox and was able to dismiss it by clicking 2x - once to focus window and then once to actually click in the window. Did this multiple times in a row with no errors.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- T5188
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1983 Build 1984: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
| webroot/rsrc/js/core/behavior-global-drag-and-drop.js | ||
|---|---|---|
| 45 ↗ | (On Diff #24299) | This calls a private (underscore) method of another class. Can JX.PhabricatorDragAndDropFileUpload handle this instead? |
Comment Actions
That is, just implement the "clicking the drop target cancels an ongoing drag" for all drop targets.
Comment Actions
Okay, I'll try that. I figured it belonged in the "global" behavior that uses the core bit since this doesn't make sense for non-global drag and drops. I think if its implemented generally though it will cause no actual problems.
Comment Actions
put it in the core behavior so we aren't abusing private methods
also update celerity map
| webroot/rsrc/js/core/DragAndDropFileUpload.js | ||
|---|---|---|
| 78 | Just remove e so the signature is function (). I like these "documentation" parameters, but enabling this rule caught at least a handful of real errors (variables which were declared/assigned but not used) and getting those actual issues cleaned up seemed worth getting yelled at about these sort-of-nice-to-have-but-not-technically-necessary parameters. Ideally we'd detect the other stuff but ignore these, but we'd have to fork jshint to support that. Or maybe add e.kill(), it seems like we should be swallowing these events? Maybe like: if (this._depth) {
e.kill();
this._updateDepth(...);
} | |
Comment Actions
do the e.kill() version. also add a '_dragging' member variable, cuz this 'click' handler should do absolutely nothing if / when we are not dragging.
Comment Actions
missed the this._depth tip. kill _dragging variable and just use _depth. re-tested and its good