Page MenuHomePhabricator

Fix transaction comment bug
ClosedPublic

Authored by btrahan on Feb 13 2014, 10:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Sep 26, 9:19 AM
Unknown Object (File)
Thu, Sep 5, 12:15 PM
Unknown Object (File)
Sep 4 2024, 6:52 PM
Unknown Object (File)
Sep 4 2024, 10:06 AM
Unknown Object (File)
Sep 4 2024, 5:13 AM
Unknown Object (File)
Sep 1 2024, 6:56 PM
Unknown Object (File)
Aug 25 2024, 12:30 PM
Unknown Object (File)
Aug 21 2024, 12:31 PM

Details

Reviewers
epriestley
Maniphest Tasks
Restricted Maniphest Task
Commits
Restricted Diffusion Commit
rP5729bbc085f2: Fix transaction comment bug
Summary

form.reset() resets a form to whatever values were present when the form was loaded into the DOM. Instead, grab all the pertinent form bits and set there values to "clear". I don't think there's too much utility in putting this somewhere more general, but it could be something like DOM.clearForm(form) or something. Fixes T3629.

Test Plan

repro'd original issue

  • open legalpad doc (or your favorite application transaction powered app)
  • type in a comment but do not submit (you are creating a draft) e.g. "foo"
  • reload page and note comment appears e.g. "foo"
  • change comment e.g. "foobar"
  • submit comment
  • BUG - after submission, the comment reverts to the comment at initial page load e.g. "foo"

then after this patch, I can't repro it anymore with these steps - the comment is correctly blank

NOTE: this will need to be tested with more complicated forms.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley added inline comments.
webroot/rsrc/js/application/transactions/behavior-transaction-list.js
135โ€“141

This seems a tiny bit sketch to me (mostly the "select-one", which I think might be some kind of unusual magic), but we can clean it up when Differential/Maniphest start using it if there's an issue, since they actually have selects.

webroot/rsrc/js/application/transactions/behavior-transaction-list.js
135โ€“141

yeah, i was googling around for how to do this, thought this was basically crap, but read

http://www.w3schools.com/jsref/prop_select_type.asp

shipitquick

webroot/rsrc/js/application/transactions/behavior-transaction-list.js
142

test

hello again inlines

we missed you :)