Commit graph

147 commits

Author SHA1 Message Date
Bartosz Dziewoński 6a59149132 Ignore LRM and RLM in more places in the timestamp
We previously ignored them before timezone indicator (e9c401e3aa),
but they can end up in other places too, e.g. after the time.

Now we ignore them after every token. This is way overkill, but it
shouldn't hurt.

Bug: T308448
Change-Id: I20f7aaa34dba23f2a2faf1be258c1aea32ab770f
2022-05-17 02:00:22 +02:00
jenkins-bot fd6297b8a8 Merge "Remove data-mw-comment-name attribute from subscribe links" 2022-03-29 21:01:25 +00:00
jenkins-bot 081fa1008a Merge "Use Sanitizer::stripAllTags() when generating notification snippets" 2022-03-28 22:42:39 +00:00
Ed Sanders 48fdcf1056 Remove data-mw-comment-name attribute from subscribe links
The HeadingItem's name is now present in the DOM, so just traverse
to it and use that instead.

Change-Id: If28e1588742513d606e3d8fcfb259b85acc0a873
2022-03-28 22:30:59 +01:00
jenkins-bot a07871d7d7 Merge "Add item name to the JSON output of HeadingItem" 2022-03-25 11:51:42 +00:00
David Lynch 71049f4ce8 Add item name to the JSON output of HeadingItem
The name is needed for the topic subscription API

Bug: T285971
Change-Id: Iedbebdfd65d03ab01b22b35781803655749aa269
2022-03-25 11:45:50 +00:00
jenkins-bot 334e1a3f33 Merge "Fix parsing of non-English titles in tests" 2022-03-23 01:02:12 +00:00
Bartosz Dziewoński c5375e05b9 CommentUtils: Fix isSingleCommentSignedBy() with empty heading
Change the order of checks to ensure that we have at least one comment
before we try comparing ranges, to avoid issues with empty headings
having collapsed ranges. It should be a tiny bit faster this way, too.

Bug: T304377
Change-Id: I59ad30cfc075dcec882e048d2d199744efec2114
2022-03-22 00:12:42 +01:00
Bartosz Dziewoński b68832ace0 Fix parsing of non-English titles in tests
We were calling Title::newFromText() before setupEnv(), which meant
that the title for each test case was parsed using the default rules
for English, rather than the rules for the specified wiki.

This only makes a practical difference for tests with self-links.
Changed the only such test to demonstrate the fix.

Change-Id: I45561f1c9f0d149e2b743f0000b742bf6fc014af
2022-03-18 18:24:07 +00:00
Bartosz Dziewoński 01b253c5b6 Don't allow the root node to be treated like a comment frame
Also fix a bug where headings would be ignored while checking for
comment frames. See task for detailed explanation.

Bug: T303396
Change-Id: I6495826b4b050ea80680e0798ac6ab4497a7c09e
2022-03-10 17:45:08 +00:00
Bartosz Dziewoński 77614a2d02 tests: Fix root node / container handling
Since times immemorial, and for reasons lost to history, our test code
was adding an extra <div> wrapper before parsing the HTML used for
tests. This wasn't a problem, until now, because I want to add some
tests for T303396 that need to check that the *real* wrappers present
in some test cases are handled correctly.

Changes to test cases mostly remove a leading "0/" from serialized
ranges, corresponding to removing the extra wrapper.

Change-Id: Ia50e3590538c8cd274b02d2a937ba1a3fbb4ac89
2022-03-10 18:43:58 +01:00
jenkins-bot dd24b0edcd Merge "Improve handling for comments after fake headings using wikitext ;" 2022-03-10 16:21:18 +00:00
jenkins-bot 76478dda26 Merge "Move signatureScanLimit to a constant in JS" 2022-03-10 00:22:14 +00:00
jenkins-bot bd43c2a139 Merge "Add test case for fake headings using wikitext ;" 2022-03-10 00:17:49 +00:00
jenkins-bot 3c91a800ed Merge "Improve detecting already signed comments" 2022-03-02 14:14:13 +00:00
jenkins-bot e15ccb8a07 Merge "Highlight all comments since the oldest in a thread bundle" 2022-02-28 23:24:12 +00:00
Ed Sanders dc8b4e8d4f Highlight all comments since the oldest in a thread bundle
For topic subscriptions, further restrict this to comments
in the same thread.

Bug: T302014
Change-Id: Ifba218871122901031a891034e709b886fc406da
2022-02-28 21:58:10 +00:00
jenkins-bot e4fa34f025 Merge "Don't insert comment markers inside replaced elements (like <video>)" 2022-02-28 17:16:11 +00:00
Bartosz Dziewoński 1e3ce9c88a Don't insert comment markers inside replaced elements (like <video>)
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.

Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
2022-02-22 15:11:34 +00:00
Bartosz Dziewoński 8a2715bdd5 Move signatureScanLimit to a constant in JS
Change-Id: Ieb60c148fd060ab62e4a493e2d0dff6c051f945c
2022-02-21 22:42:14 +01:00
Bartosz Dziewoński 0ecc8a4c05 Improve detecting already signed comments
Previously, we required a signature at the end of the comment.
This was a pretty rough heuristic that did not correctly handle
many comments that we would consider entirely properly signed
in CommentParser (e.g. comments wrapped in formatting like
<small>…</small>, comments with a post-scriptum or in parentheses,
or comments generated by various templates).

Now we process the user input using the same code that adds reply
links, and only add a signature when we detect that there really
isn't a signature (including template-generated), or if the signature
is in the wrong place and would result in the reply link showing up
in the wrong place as well (not at the end of the comment).

Bug: T278442
Bug: T268558
Bug: T278355
Bug: T291421
Bug: T282983
Change-Id: I46b6110af328ebdf93b7dfc2bd941e04391a1599
2022-02-21 21:21:26 +00:00
Bartosz Dziewoński 4244418e56 Don't detect comments within references
Bug: T301213
Change-Id: Ifd5198651c8ed0ce53379fb5e35938089cd54a09
2022-02-21 19:57:44 +00:00
Bartosz Dziewoński e414d1acaf Improve handling for comments after fake headings using wikitext ;
Bug: T265964
Change-Id: I77db68928c5426fd885a277eec52c6e164d559bb
2022-02-11 23:35:32 +00:00
Bartosz Dziewoński 62766e846c Add test case for fake headings using wikitext ;
Bug: T265964
Change-Id: I31db66dc011b3d13dd46426f15286c5b3b5c9254
2022-02-12 00:35:02 +01:00
Bartosz Dziewoński 91e1bb15cc Use Sanitizer::stripAllTags() when generating notification snippets
It adds white-space between block tags and strips invisible tags.

It may be slightly slower (it takes HTML as input rather than DOM, so
we need to serialize the HTML first and then call it, rather than only
find and concatenate text nodes), but the difference is negligible,
and it seems better to use this method than to try to re-implement it.

Test runtime went from ~9.0s to ~9.5s locally, when testing using:
  php tests/phpunit/phpunit.php \
  extensions/DiscussionTools/tests/phpunit/ThreadItemTest.php \
  --filter getText

Bug: T219138
Change-Id: I0cb89ebd2160e1ef499b78573c6688f493a4c42f
2022-02-10 22:23:24 +01:00
Bartosz Dziewoński 15f0867b75 Limit where whitespaceParsoidHack() is used
* We don't need it anywhere in JS, since we're not sending that HTML
  to Parsoid.
* We only need it on the nodes directly containing our reply lines in
  PHP, not all over the place.

Change-Id: I0a04388225f32654dda2f599442cd27a303b5d0a
2022-01-29 22:42:46 +00:00
Bartosz Dziewoński 110a59200f One more tweak for comparing comment ranges to transclusion/DOM ranges
When we encounter a node that doesn't represent comment contents, e.g.:
* a [reply] link we inserted (T297034#7641334)
* an {{outdent}} template (see changed test case)

…we should ignore it together with its descendants (like in
Parser#nextInterestingLeafNode), instead of processing descendants
and possibly detecting comment contents in them.

Follow-up to 8de940b587,
72b9c2c6f5.

Bug: T297034
Change-Id: Ib2fa40c5fa389572b0e88ef558728fa06e3621b0
2022-01-24 17:42:18 +00:00
jenkins-bot 3a4a6bff16 Merge "Add another test case for transclusions overlapping comments" 2022-01-21 22:25:46 +00:00
Bartosz Dziewoński 4e4e9f1c80 Add another test case for transclusions overlapping comments
Prior to 8de940b5, the comments on this page would not be marked as
partially transcluded.

Bug: T298408
Change-Id: Ib7eb8b4113151048c0e778b3530600d98dd8f705
2022-01-17 23:52:02 +01:00
Bartosz Dziewoński b7cbd714ca Add tests for bullet indentation
Bug: T259864
Change-Id: If38016564b67ee7217fe7328b40973aa244ff467
2022-01-14 00:27:04 +00:00
Ed Sanders f80ff74fc6 Handle selflinks by returning the current page's title
Bug: T287818
Change-Id: I67f10ac9976581279d1e6a477e90d55875ebab20
2022-01-12 21:18:04 +00:00
Ed Sanders 1fed7115f4 Tests: Add original titles to test cases
These are not used for anything yet, but soon the parser will
want to know the title of the page it is parsing.

Change-Id: I02fa5d63fae78f3e92032d93bc27ac5c744faecb
2022-01-12 22:16:03 +01:00
Bartosz Dziewoński 72b9c2c6f5 Ignore some invisible nodes when looking for comment frames
Reimplement getFullyCoveredSiblings() using compareRanges(), which
checks basically the same thing, but works better and I like it more.

Bug: T297034
Change-Id: I33dc1d088bdee984064315290e378bfbfa830b10
2022-01-11 17:01:53 +00:00
Bartosz Dziewoński 8d51ce32b6 Test cases for adding replies outside of frames
Bug: T297034
Change-Id: Ida57b2638d3434c9b47b120942348ebfe0f96d37
2022-01-11 17:01:47 +00:00
Bartosz Dziewoński 8de940b587 Improve detecting transcluded comments again
Previously: 569db3603c (2020-06).

Unfortunately we've found cases where the previous implementation
doesn't work correctly, resulting in comments being added to the wrong
pages or page corruption.

Bug: T289873
Bug: T298051
Change-Id: Id867b3005ebc46906d6df852a525fcaec9e6b19b
2022-01-11 16:07:44 +00:00
Bartosz Dziewoński 492dbd7847 Fix inserting comment start markers when they're outside of any wrappers
Comment boundaries are stored as a DOM parent node plus a child index.
Because of that, inserting anything into the DOM before a comment –
such as another comment's start/end markers – would cause us to insert
subsequent comments' markers into the wrong places.

This issue didn't affect many pages, because usually any parent node
would have just one comment in it. Only pages with comment boundaries
outside of any wrappers (directly inside the root node) were broken.

Just process the list in reverse to fix this.

Bug: T298096
Change-Id: Iccffc36b71e9fcf3d72c4db2b9459d39042f7a2d
2022-01-11 16:07:37 +00:00
Bartosz Dziewoński d24b04ee71 Test case for several bugs from it.wp
Servers as another test case for partially transcluded comments,
and a test case for comment start markers placed outside of
paragraphs.

Bug: T298051
Bug: T298096
Change-Id: Id07d2f57708c037578cb653c83848490c9a15fc6
2022-01-05 23:17:51 +00:00
Bartosz Dziewoński 4da9a13c45 Test cases for partially transcluded comments
Bug: T289873
Change-Id: I115a46eb4858dccd6056534d727f88d8513b391b
2021-12-17 00:55:33 +00:00
Bartosz Dziewoński 83ba496919 Avoid splitting about-groups starting with an empty <span>
Usually this isn't a problem, because the comments are marked as
template-generated and we don't allow replying to them. But we had a
special case where we were trying to skip over some invisible
elements, which was causing us to skip into the middle of the
about-group in some cases. When Parsoid sees that, it serializes the
contents twice.

Bug: T290940
Change-Id: I9fe0b8d43ab874ccef371990799f77bfc46bc954
2021-11-15 16:03:38 +00:00
Bartosz Dziewoński e6de1c0462 Test case for splitting about-groups starting with an empty <span>
Bug: T290940
Change-Id: I632d351cf61980a48c0a16c4bdb3656dac83a584
2021-11-15 16:03:28 +00:00
Bartosz Dziewoński c1f4668806 Change CommentParser and ImmutableRange to use offsets in codepoints instead of bytes
The PHP DOM extension measures lengths and offsets in Unicode codepoints.
Our PHP code used UTF-8 bytes, causing some offsets to be slightly off.
Now it mostly uses Unicode codepoints as well (we're forced to use bytes
in a few places, because preg_match returns offsets in bytes).

In practice, this had no visible effect to the user. It caused the
markers `<span data-mw-comment-end="..."></span>` to be placed at
the end of their container instead of the correct position when the
timestamp contained multibyte characters (e.g. "ź" in Polish); but
the correct position is usually at the end of the container anyway.

In the test cases, the only difference is placing these markers before
a trailing line break inside `<p>...</p>` tags rather than before it.

The patch also accidentally fixes another bug, where element nodes
with no children (mostly <img>) were incorrectly excluded when calling
cloneContents(), because they were treated as if they were text nodes.

Change-Id: Iccdccf1078598f4b62cab96225e9c85a4c0e93ee
2021-09-27 19:04:16 +00:00
jenkins-bot abd6c2fedd Merge "Enhance Echo user talk edit and mention notifications" 2021-09-24 02:17:57 +00:00
Bartosz Dziewoński 435b0c65c7 Enhance Echo user talk edit and mention notifications
If the user talk edit or mention coincides with exactly one new comment:
* Change the primary link to be a direct link to the comment
* Add a text snippet to notifications that don't already include one
  (user talk edits that are not new sections).

This is done for all such notifications, regardless of whether anyone
has topic subscriptions enabled.

Bug: T281590
Bug: T253082
Change-Id: I98fbca8e57845cd7c82ad533c393db953e4e5643
2021-09-20 15:05:42 +02:00
Bartosz Dziewoński a6a547f2b2 Add some tests covering ThreadItem::getHTML() and related methods
* ThreadItem::getText
* CommentItem::getBodyText (used when generating notifications)
* ThreadItem::getHTML (may soon be used in API)
* CommentItem::getBodyHTML (may soon be used in API)
* ImmutableRange::cloneContents (the common implementation for all
  of the above)

The outputs are only lightly reviewed. This is mostly meant to
document the current behavior rather than the expected behavior,
to avoid making unintentional changes while refactoring.

Change-Id: I14471ee4969aa3d0b5577d9de2a6d4462fab4d09
2021-08-24 07:54:09 +02:00
Bartosz Dziewoński 47510a22f3 EventDispatcher: Fix ignoring level 3+ headings
The code (prior to d25825a754) assumed
that level 3+ headings would always follow a level 2 heading or the
placeholder heading, but we don't generate a placeholder heading if
there are no comments in section zero.

Add more tests to confirm that comments under level 3+ headings (that
are not sub-headings of level 2), and level 1 headings, are ignored
when generating notifications, and do not mess with normal headings.

Bug: T288775
Change-Id: Ic57b56752a4797cb01234f66e0ed7b849752bd70
2021-08-16 15:42:06 +00:00
jenkins-bot 56f4dbbf86 Merge "Test cases for interactions with events generated by base Echo" 2021-08-03 23:18:46 +00:00
jenkins-bot d14f31fd08 Merge "Improve notifications for comments posted in close succession" 2021-08-03 23:17:55 +00:00
jenkins-bot 352a4d0555 Merge "Test cases for comments posted in close succession" 2021-08-03 20:44:02 +00:00
jenkins-bot 1f4706a308 Merge "Recognize links to add a new topic that use Special:NewSection" 2021-08-02 17:42:30 +00:00
Bartosz Dziewoński 80704b6e80 Test cases for interactions with events generated by base Echo
Adding test cases in a separate commit to make it easier to review how
the test results change after I98fbca8e.

* For mentions, the 'mentioned-users' extra parameter is copied to our
  event (which is then used to avoid duplicate notifications).
* For user talk page edit, nothing special happens right now (we use
  the target page title to avoid duplicate notifications, but this is
  not apparent from the test case, since page titles are not present).

Bug: T281590
Bug: T253082
Change-Id: I153e7735f63f1e2643ed881281d807313cd699c3
2021-08-01 12:27:33 +02:00