Page MenuHomePhabricator

Fix transaction comment bug
ClosedPublic

Authored by btrahan on Feb 13 2014, 10:53 PM.
Tags
None
Referenced Files
F15533475: D8220.id19553.diff
Wed, Apr 23, 7:55 PM
F15489760: D8220.id.diff
Fri, Apr 11, 12:03 PM
F15428880: D8220.diff
Mar 23 2025, 10:57 PM
F15427121: D8220.diff
Mar 23 2025, 1:00 PM
F15425481: D8220.id.diff
Mar 23 2025, 4:47 AM
F15391973: D8220.id19553.diff
Mar 15 2025, 12:36 PM
Unknown Object (File)
Feb 17 2025, 1:23 PM
Unknown Object (File)
Feb 17 2025, 1:23 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 :)