mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 23:32:40 +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 |
||
|---|---|---|
| .. | ||
| __tests__ | ||
| control | ||
| behavior.js | ||
| Cookie.js | ||
| DOM.js | ||
| History.js | ||
| JSON.js | ||
| Mask.js | ||
| Request.js | ||
| Resource.js | ||
| URI.js | ||
| Vector.js | ||
| Workflow.js | ||