mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-30 17:30:59 +01:00
e8126fa958
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 |
||
|---|---|---|
| .. | ||
| css | ||
| externals | ||
| image | ||
| js | ||
| swf | ||