Commit graph

59 commits

Author SHA1 Message Date
Bartosz Dziewoński e9b583d1c3 parser: Improve merging multiple comments on one line
Now also works if the "follow-up" comment is wrapped in e.g. `<small>`.

Change-Id: Ic37cb6afdb42021f109a1818f5c4299d907ed094
2020-03-14 13:34:42 +00:00
Bartosz Dziewoński 2a0f53fed9 Replace the closest() polyfill
If we can't use the native closest() due to lack of browser support,
then I'd rather have a simple loop that implements the functionality
we need, instead of a fallback to another native method with limited
browser support and experimental implementations.

Change-Id: I0bf84aa25fc398e329b533afb28317d19716d57a
2020-03-09 13:28:14 +00:00
Bartosz Dziewoński 606d6b34ec Add reply links at the end of a line, even if the signature is in the middle
Previously they were added at the end of the text node containing the
timestamp, which was usually the end of the line, but not always.

And also fix the same problem for inserting the actual replies (or
reply widgets). This replaces an undocumented hack that prevented our
own reply links from triggering this bug (without it, the reply widget
would be inserted before the reply link rather than after).

While we're here, remove unintentional spacing that appeared before
some reply links, caused by trailing whitespace in text nodes.

Add tests for all of the above.

Bug: T245695
Change-Id: I354b63e2446bb996176a2e3d76abf944127f307e
2020-03-02 21:39:37 +01:00
Bartosz Dziewoński 85b2cf00b1 modifier: Fix IE 11 incompatibility due to 'parentElement'
On IE 11, the 'parentElement' property is only supported on element
nodes, not on text nodes.

https://developer.mozilla.org/en-US/docs/Web/API/Node/parentElement#Browser_compatibility

There's no reason to use it here, 'parentNode' is the same for the
nodes we're concerned with.

Also remove the use in code adapted from MDN to avoid repeating this
issue in the future.

Bug: T246565
Change-Id: I0120feb3737c462f2a64e4ec084249a0fd57d0f0
2020-03-02 18:14:35 +01:00
Bartosz Dziewoński d068d2ef2c Clean up the interface after discarding a reply
Bug: T245574
Change-Id: I016a7a5c44e0d15a153143177976cceb8d6d3d1b
2020-02-27 18:09:34 +01:00
Bartosz Dziewoński 890588f36a Pick reply insertion point based on parser tree, not DOM tree
I don't like that I had to special-case `<p>` tags (top-level
comments) in this code. I feel like it should be possible to handle
top-level comments and replies in a generic way, but I couldn't find
a way to do it that actually worked.

Notes about changes to the behavior, based on the test cases:

* Given a top-level comment A, if there was a "list gap" in the
  replies to it: previously new replies would be incorrectly added at
  the location of the gap; now they are added after the last reply.
  (T242822)

  Example: "pl", comment at "08:23, 29 wrz 2018 (CEST)"

* Given a top-level comment A and a reply to it B that skips an
  indentation level: previously new replies to A would be added with
  the same indentation level as B; now they are added with the
  indentation level of A plus one. (The old behavior wasn't a bug, and
  this is an accidental effect of other changes, but it seems okay.)

  Example: "pl", comment at "03:22, 30 wrz 2018 (CEST)"
    and reply at "09:43, 30 wrz 2018 (CEST)"

* Given a top-level comment A, a reply to it B, and a following
  top-level comment C that starts at the same indentation level as B:
  previously new replies to A would be incorrectly added in the middle
  of the comment C, due to the DOM list structure; now they are added
  before C. (T241391)

  (It seems that comment C was supposed to be a multi-line reply that
  was wrongly indented. Unfortunately we have no way to distinguish
  this case from a top-level multi-line comment that just happens to
  start with a bullet list.)

  Example: "pl", comments at "03:36, 24 paź 2018 (CEST)",
    "08:35, 24 paź 2018 (CEST)", "17:14, 24 paź 2018 (CEST)"

* In the "en" example, there are some other changes where funnily
  nested tags result in slightly different results with the new code.
  They don't look important.

* In rare cases, we must split an existing list to add a reply in the
  right place. (Basically add `</ul>` before the reply and `<ul>`
  after, but it's a bit awkward in DOM terms.)

  Example: split-list.html, comment "aaa"; also split-list2.html
    (which is the result of saving the previous reply), comment "aaa"

* The modifier can no longer generate DOM that is invalid HTML, fixing
  a FIXME in modifier.test.js (or at least, it doesn't happen in these
  test cases any more).

Bug: T241391
Bug: T242822
Change-Id: I2a70db01e9a8916c5636bc59ea8490166966d5ec
2020-01-23 21:13:12 +01:00
Ed Sanders ccbbf386d2 Live preview
Change-Id: I1654e95e94686b27818cbc465d5a9e8600404634
2019-12-12 23:59:24 +00:00
Ed Sanders 9733f72354 Use Parsoid hack to remove whitespace from replies
Change-Id: I1ad0f5498c3c409df6d4eb2d400a0f714dff9908
2019-11-19 21:24:33 +00:00
Ed Sanders 16de8fb288 Rewrite list modifier in plain JS and move to separate file
Change-Id: Idc4867f1fe266de21e8964e5887aabd545d6c7ee
2019-11-05 14:13:18 +00:00