Page MenuHomePhabricator

Fix transaction comment bug
ClosedPublic

Authored by btrahan on Feb 13 2014, 10:53 PM.
Tags
None
Referenced Files
F18791485: D8220.id19553.diff
Thu, Oct 16, 1:59 PM
F18737623: D8220.diff
Wed, Oct 1, 11:41 AM
F18735972: D8220.id19559.diff
Wed, Oct 1, 4:18 AM
F18693512: D8220.id.diff
Sat, Sep 27, 1:26 AM
F18410195: D8220.id.diff
Aug 30 2025, 4:45 AM
F18395292: D8220.diff
Aug 29 2025, 11:01 AM
F18103735: D8220.id.diff
Aug 10 2025, 7:31 AM
F18103734: D8220.id19553.diff
Aug 10 2025, 7:31 AM

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 :)