Page MenuHomePhabricator

Fix Passphrase Credential dialog
ClosedPublic

Authored by epriestley on Apr 13 2016, 12:32 AM.
Tags
None
Referenced Files
F14057436: D15691.diff
Sun, Nov 17, 3:13 AM
F14044587: D15691.id37813.diff
Tue, Nov 12, 6:14 PM
F14037362: D15691.id37808.diff
Sun, Nov 10, 3:26 PM
F14027157: D15691.id.diff
Fri, Nov 8, 5:43 AM
F14025648: D15691.id37817.diff
Thu, Nov 7, 7:20 PM
F14025569: D15691.id37813.diff
Thu, Nov 7, 6:52 PM
F14025560: D15691.id37817.diff
Thu, Nov 7, 6:50 PM
F14025542: D15691.diff
Thu, Nov 7, 6:46 PM
Subscribers

Details

Summary

Fixes T10772, not sure why this fails, but reverting the code back to old dialog call works.

Test Plan
  • Try to add a new credential when importing a repository.
  • Also created a new credential normally, via Passphrase.
  • Also edited a credential.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Fix Passphrase Credential dialog.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.

Let me see if I can figure out what's going on here, these are supposed to be equivalent.

I was writing 1 Big Text

boss = I took a nap

T.T

I'm trying to live vicariously. I'll never play a game like Dark Souls. :( I am casual gamer dad now.

epriestley edited reviewers, added: chad; removed: epriestley.

I think I have a sliiightly cleaner-ish fix for this, one sec...

Issue is that newDialog() sets an explicit URI, while new AphrontDialogView() does not.

The explicit URI drops GET parameters. It's sort of good that we do this so that everything is more explicit, this is kind of a bug with the existing form.

Doing it the implicit way could run into other issues with GET parameters conflicting with other GET parameters in unusual cases, which would be more weird/harder to figure out than this.

Put "type" parameter in the form explicitly.

chad edited edge metadata.
This revision is now accepted and ready to land.Apr 13 2016, 3:09 AM
This revision was automatically updated to reflect the committed changes.