Homec4science

Don't mangle inline comments with tables in them in Differential

Authored by epriestley <git@epriestley.com> on Sep 11 2013, 00:31.

Description

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

Details

Committed
epriestley <git@epriestley.com>Sep 11 2013, 00:31
Pushed
aubortJan 31 2017, 17:16
Parents
rPH51eb8a301aa2: Clean up Diffusion repository list
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHe8126fa95858: Don't mangle inline comments with tables in them in Differential (authored by epriestley <git@epriestley.com>).Sep 11 2013, 00:31