1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-30 10:42:41 +01:00
phorge-phorge/webroot/rsrc/js/application/differential
epriestley e8126fa958 Don't mangle inline comments with tables in them in Differential
Summary:
Fixes T3814. Broadly, remarkup tables in inline comments did not work properly. I ran into several messes here and cleaned up some of them:

  - Some of this code is doing `JX.$N('div', {}, JX.$H(response.markup))`, to turn an HTML response into a node, passing that around, and then doing junk with it. This is super old and gross.
    - The slightly more modern pattern is `JX.$H(response.markup).getFragment().firstChild`, but this is kind of yuck too and not as safe as it could be.
    - Introduce `JX.$H(response.markup).getNode()`, which actually expresses intent here. We have a bunch of `getFragment().firstChild` callsites which should switch to this, but I didn't clean those up yet because I don't want to test them all.
    - Switch the `JX.$N('div', {}, JX.$H(response.markup))`-style callsites to `JX.$H(response.markup).getNode()`.
  - `copyRows()` is too aggressive in finding `<tr />` tags. This actually causes the bug in T3814. We only want to find these tags at top level, not all tags. Don't copy `<tr />` tags which belong to some deeper table.
  - Once this is fixed, there's another bug with mousing over the cells in tables in inline comments. We select the nearest `<td />`, but that's the cell in the remarkup table. Instead, select the correct `<td />`.
  - At this point, these last two callsites were looking ugly. I provided `findAbove()` to clean them up.

Test Plan: Created, edited, deleted, moused over, and reloaded a revision with inline comments including remarkup tables. Used "Show more context" links.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3814

Differential Revision: https://secure.phabricator.com/D6924
2013-09-10 15:31:32 -07:00
..
behavior-accept-with-errors.js Toggle accept warnings on reload 2012-12-17 12:31:00 -08:00
behavior-add-reviewers-and-ccs.js Show add reviewer typehead when user selects resign as a reviewer. 2013-07-10 11:05:53 -07:00
behavior-comment-jump.js Use JsShrink if jsxmin is not available 2013-05-18 17:04:22 -07:00
behavior-comment-preview.js Use JsShrink if jsxmin is not available 2013-05-18 17:04:22 -07:00
behavior-diff-radios.js Bring Javelin into Phabricator via git submodule, not copy-and-paste 2011-05-08 13:20:10 -07:00
behavior-dropdown-menus.js See https://github.com/facebook/phabricator/issues/262 2013-07-08 19:56:43 -07:00
behavior-edit-inline-comments.js Don't mangle inline comments with tables in them in Differential 2013-09-10 15:31:32 -07:00
behavior-keyboard-nav.js Use JsShrink if jsxmin is not available 2013-05-18 17:04:22 -07:00
behavior-populate.js Use JsShrink if jsxmin is not available 2013-05-18 17:04:22 -07:00
behavior-show-all-comments.js Use JsShrink if jsxmin is not available 2013-05-18 17:04:22 -07:00
behavior-show-field-details.js Further improve unit/lint rendering 2012-05-01 10:15:56 -07:00
behavior-show-more.js Don't mangle inline comments with tables in them in Differential 2013-09-10 15:31:32 -07:00
behavior-toggle-files.js Better JS for Differential File Collapsing Undo 2013-08-13 16:14:26 -07:00
behavior-user-select.js Enable selecting text in Differential shield and gap 2012-09-20 15:20:47 -07:00
DifferentialInlineCommentEditor.js Don't mangle inline comments with tables in them in Differential 2013-09-10 15:31:32 -07:00