Commit graph

84 commits

Author SHA1 Message Date
Bartosz Dziewoński ad04b24ffd Create a hidden revision tag for talk page comments
Bug: T262107
Depends-On: I21159d03eebaf46ad94f4273ba698a59b8019185
Change-Id: Iceddfaf6a4bcc5e8b5c85c8cd5638bf14aa7db03
2021-08-16 15:42:51 +00: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
Bartosz Dziewoński b46893eb7d Remove pointless uses of preserveWhiteSpace property
This DOMDocument property has no effect, because we do not use
DOMDocument methods for parsing HTML, but rather DOMUtils::parseHTML()
provided by Parsoid.

Change-Id: I1d9e73e53f2d44f41cf9dcda4f06ac8647671096
2021-08-09 23:45:48 +02:00
jenkins-bot 10c23d0eb1 Merge "Deal with document body consistently" 2021-08-06 03:08:28 +00:00
Bartosz Dziewoński 8de8d80cde Deal with document body consistently
Use `DOMCompat::getBody( ... )` as a nicer getter than
`->getElementsByTagName( 'body' )->item( 0 )`.

Remove overly defensive checks and redundant annotations on its
return value. Since we're dealing with HTML documents throughout,
the document body is guaranteed to exist.

We previously needed some of them to convince Phan when it thought
the body may be null, but this seems to no longer be needed.

Change-Id: If7aee7b6adbfa78269c7ba28b26a6eaa21fe935b
2021-08-03 15:12:55 +02: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
Bartosz Dziewoński 78cb03c471 Test cases for comments posted in close succession
Adding test cases in a separate commit to make it easier to review how
the test results change.

As expected, in every case, no notifications are generated right now.

Bug: T285528
Change-Id: I25308754112c521d2db8c54ef0c82373456d9e31
2021-08-01 12:27:33 +02:00
C. Scott Ananian 25272e7a4a Don't refer directly to PHP dom extension classes; avoid nonstandard behavior
These changes ensure that DiscussionTools is independent of DOM
library choice, and will not break if/when Parsoid switches to an
alternate (more standards-compliant) DOM library.

We run `phan` against the Dodo standards-compliant DOM library,
so this ends up flagging uses of non-standard PHP extensions to
the DOM.  These will be suppressed for now with a "Nonstandard DOM"
comment that can be grepped for, since they will eventually
will need to be rewritten or worked around.

Most frequent issues:

* Node::nodeValue and Node::textContent and Element::getAttribute()
can return null in a spec-compliant implementation.  Add `?? ''` to
make spec-compliant results consistent w/ what PHP returns.

* DOMXPath doesn't accept anything except DOMDocument.  These uses
should be replaced with DOMCompat::querySelectorAll() or similar
(which end up using DOMXPath under the covers for DOMDocument any way,
but are implemented more efficiently in a spec-compliant
implementation).

* A couple of times we have code like:
  `while ($node->firstChild!==null) { $node = $node->firstChild; }`
and phan's analysis isn't strong enough to determine that $node is still
non-null after the while.  This same issue should appear with DOMDocument
but phan doesn't complain for some reason.

One apparently legit issue:

* Node::insertBefore() is once called in a funny way which leans on
the fact that the second option is optional in PHP.  This seems to be
a workaround for an ancient PHP bug, and can probably be safely
removed.

Bug: T287611
Bug: T217867
Change-Id: I3c4f41c3819770f85d68157c9f690d650b7266a3
2021-07-30 18:15:40 -04:00
C. Scott Ananian 5203d30ea6 Use DOMCompat::newDocument() to create a new Document
For compatibility with Parsoid's document abstraction (Parsoid may
switch to an alternate DOM library in the future), don't explicitly
create a new document object using `new DOMDocument`; instead use
the Parsoid wrapper `DOMCompat::newDocument()`.  This ensures that
the Document object created will be compatible with Parsoid.

There are a number of other subtle dependencies on the PHP `dom`
extension in DiscussionTools, like explicit `instanceof` tests; those
will be tweaked in a follow-up patch
(I3c4f41c3819770f85d68157c9f690d650b7266a3) since they do not affect
correctness so long as Parsoid is aliasing Document to a subclass of
the built-in DOMDocument.  Similarly, the Phan warnings we suppress
do not cause runtime errors (because of the fixes included in
c5265341afd9efde6b54ba56dc009aab88eff83c) but phan will be happier
once the follow-up patch lands and aligns all the DOM types.

Bug: T287611
Depends-On: If0671255779571a91d3472a9d90d0f2d69dd1f7d
Change-Id: Ib98bd5b76de7a0d32a29840d1ce04379c72ef486
2021-07-30 18:15:11 -04:00
Bartosz Dziewoński d0e4aeaecb Fix notifications when new comment is under subheading
The user interface only allows you to subscribe to level 2 headings.
But we would generate events for whatever heading was the closest,
If it was e.g. level 3, no one would receive that notification.

Now we generate events for the closest level 2 heading, or we don't
generate the event at all if there isn't one (if the only headings are
of level 3 and below, or level 1, or if the comment is added before
the first heading on the page).

Bug: T286736
Change-Id: Iae99853070e353ab81c9cc29ef1d53c877adfc66
2021-07-24 05:28:10 +02:00
Bartosz Dziewoński 801b57b0f4 Add PHPUnit integration tests for EventDispatcher
Bug: T286608
Change-Id: I711483be80d455f4439e96d37844ee4552619a92
2021-07-24 05:28:04 +02:00
libraryupgrader b0884b177c build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0

npm:
* postcss: 7.0.35 → 7.0.36
  * https://npmjs.com/advisories/1693 (CVE-2021-23368)
* glob-parent: 5.1.1 → 5.1.2
  * https://npmjs.com/advisories/1751 (CVE-2020-28469)
* trim-newlines: 3.0.0 → 3.0.1
  * https://npmjs.com/advisories/1753 (CVE-2021-33623)

Change-Id: I7a71e23da561599da417db3b3077b78d91173bbc
2021-07-22 16:29:04 +00:00
Bartosz Dziewoński 9c8d709b8a Use placeholder localisation messages in CommentFormatter tests
Otherwise they will fail whenever translations are updated
(and they are failing right now).

Change-Id: I849c57b86d36fb6c7739cc31a74df741e08462f4
2021-06-02 21:46:36 +02:00
libraryupgrader 12fb65b9f1 build: Updating composer dependencies
* mediawiki/mediawiki-codesniffer: 35.0.0 → 36.0.0
* php-parallel-lint/php-parallel-lint: 1.2.0 → 1.3.0

Change-Id: I5c152292e83e7f3441e2c08b7d0ad23ac90f194b
2021-05-05 11:14:52 +00:00
Bartosz Dziewoński 475aa80057 Fetch user's topic subscriptions on the page in a single query
Previously, we have made a query per each topic on the page.

Bug: T281000
Change-Id: I1029e62a65fc191ca37e1178ea7ffc55afafa1b9
2021-04-28 21:54:26 +00:00
Ed Sanders 722a4e5198 Avoid splitting ParserCache on user language
Bug: T280295
Change-Id: I87eab83803d24c11db4d723377bf7b40390b2e70
2021-04-21 11:57:30 +00:00
Bartosz Dziewoński 5103e651be Add tests for CommentFormatter::postprocessTopicSubscription
Change-Id: Ief9648b8805fadcc170c54b627eb669cc8b907b6
2021-04-21 11:57:25 +00:00
Bartosz Dziewoński 4bbfe6cb5d Rename CommentFormatter::addReplyLinks
Bug: T280351
Change-Id: I0d7627d63407e11cca6091f78e4d440eec6efa91
2021-04-21 11:24:03 +00:00
Bartosz Dziewoński 42ce942c86 Introduce comment "names" to identify comments across revisions/pages
The existing comment IDs can't be used to find the same comment on
a different revision or page (when it's transcluded), because they
depend on the comment's parent and its position on the page.

Comment names depend only on the author and timestamp. The trade-off
is that they can't distinguish comments posted within the same minute,
or in the same edit, so we will still need the IDs sometimes.

Prefer using comment names when replying, if they're not ambiguous.
This fixes T273413 and T275821.

Heading names depend on the author and timestamp of the oldest comment.
This way we don't have to detect changes to the heading text, but we
can't distinguish headings without any comments.

Bug: T274685
Bug: T273413
Bug: T275821
Change-Id: Id85c50ba38d1e532cec106708c077b908a3fcd49
2021-03-23 16:08:42 +00:00
Bartosz Dziewoński a103abb8ae Ignore warnings about legacy IDs in tests
Change-Id: I3c74b4e65aac9b84494917547cce7eb6a75995b4
2021-03-18 20:42:03 +01:00
Bartosz Dziewoński 44f2209abf Trim signatures when added in an empty existing node, too
Add unit tests for appendSignature().

Bug: T276612
Change-Id: Ic44c52f4d54492e092f9396c626380e2637b6f0f
2021-03-08 23:38:46 +00:00
Bartosz Dziewoński 5a07139249 CommentFormatterTest: Avoid re-serializing the HTML
The code we're testing already produces a string of serialized HTML,
no need to parse and re-serialize it.

Also, we recently learned that the precise format matters here
(T274709), and now this test *actually* covers the fix for that bug.
Follow-up to 5b26e9664b.

As a downside, this test might now spuriously fail if the format of
the output of Parsoid's XMLSerializer changes. Hopefully that won't
happen too often.

Change-Id: I69b514f545e47dcb437fb39a83edb8e2f19ed99b
2021-03-01 21:30:28 +01:00
Bartosz Dziewoński efe95494a8 Improve signature detection to handle formatting on the timestamp
Now it detect signatures generated by en.wp's {{Undated}} template,
and signatures of people who do weird stuff to the timestamps.

Bug: T275938
Change-Id: I27b07f6786ca5433a3c02a5fe68e4716d41401bb
2021-02-27 02:33:30 +01:00
Bartosz Dziewoński 1c3fada1fb Make CommentUtilsTest a proper unit test
Documentation:
https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests_for_extensions#Two_types_of_tests

We can do this because the tested methods do not depend on any globals
or on MediaWiki being installed.

In addition to being the new hotness, MediaWikiUnitTestCase allows the
test classes that use it instead of MediaWikiTestCase to start up much
faster. In my testing, running this test case individually now takes
0.35s, compared to 1.1s before.

Try:
* With new code:
  time php tests/phpunit/phpunit.php extensions/DiscussionTools/tests/phpunit/unit/CommentUtilsTest.php
* With old code:
  time php tests/phpunit/phpunit.php extensions/DiscussionTools/tests/phpunit/CommentUtilsTest.php

Change-Id: I771b1f3d101a394ee869e42547d9ae7839397752
2021-02-02 15:37:17 +01:00
Ed Sanders 2908c2808d Move Hooks::addReplyLinks to CommentFormatter
Change-Id: I9f5483cd801f48efff22cba045ae6851da9719fd
2021-02-01 22:35:04 +00:00
Ed Sanders 47aea0b160 Use tabs for indentation in JSON test files
Change-Id: I1d8f8b33b19bcff249ad08dfe687f87f5e5bf9bf
2021-01-27 00:25:15 +00:00
Bartosz Dziewoński 8f42c74985 Fix skipping to the end of paragraph, now it considers nested tags
Add yet another tree walking utility: CommentUtils::linearWalk().
Unlike TreeWalker, it allows handling the beginnings and ends of nodes
separately – kind of like parsing a XML token stream, or kind of like
VisualEditor's linear model.

(Add unit tests for this utility. The simple.html test case is copied
from [VisualEditor/VisualEditor]/demos/ve/pages/simple.html.)

Use this utility to stop skipping when we reach either a closing or
opening block node tag. Previously we'd skip over such tags inside
nested "transparent" nodes (like <a>, <del>, or apparently <font>).

Bug: T271385
Change-Id: I201a942eb3a56335e84d94e150ec2c33f8b4f4e0
2021-01-18 18:20:20 +00:00
Ed Sanders 8b71a2b5dc Load site config data in CommentFormatter tests
This fixes missing reply links in arwiki test output.

Change-Id: I24d3b8371a8343c4445c716fadf0692be0924eed
2021-01-08 23:03:33 +00:00
Ed Sanders 9ba6c3d159 CommentItem/HeadingItem: Make more constructor args required
This ensures the getters always return the promised types.

Change-Id: I1a3c909f5395463ef7a89d896ead1520b2a17509
2021-01-08 20:45:29 +00:00
Ed Sanders 0d2d3b16b8 Pass interface language object to addReplyLinks
Change-Id: I8a5562e11df3ad6430db48020d6005d0c4fd6834
2021-01-08 21:43:21 +01:00
Ed Sanders 32cd64ec6a Use Parsoid DOMCompat/DOMUtils in CommentFormatter
As CommentFormatter no longer needs HTMLFormatter, remove
the inheritance and make addReplyLinks a static method.

Testing locally this is marginally slower, going from 2.55s
to 2.9s for the CommentFormatterTest case.

Bug: T266317
Bug: T267973
Change-Id: If69749cae678a1647a138d782a32032189f55cec
2020-11-16 22:28:07 +00:00
Bartosz Dziewoński 31f6d44bf6 Move warnings stuff from CommentItem to ThreadItem
After recent changes allowing ThreadItems to have IDs, they can now
also have warnings about duplicate IDs.

Bug: T267035
Change-Id: If3edfe34e6e29741e29fac8946a3c88badc4ab7f
2020-11-02 20:07:23 +00:00
Bartosz Dziewoński 044bc50fb6 Fix some TODOs about test data
We avoided fixing these because it causes changes in just about all of
the test data, which is annoying when reviewing or blaming changes.

But the previous several commits also caused changes in just about all
of the test data, so we might as well do this too.

Change-Id: I83b64d83b6f12c04dc06c0cadff7cdd89417e137
2020-10-22 00:21:04 +00:00
Bartosz Dziewoński 3137d76f40 Connect sub-threads to their parent threads
Our threads now also contain all replies to their sub-threads.
This is similar to how sections work in MediaWiki, where the parent
section also contains the content of all the lower-level sections.
We're going to need this for notifications about replies in a thread.

Bug: T264478
Change-Id: I241fc58e2088a7555942824b0f184ed21e3a8b6f
2020-10-22 02:05:02 +02:00
Bartosz Dziewoński 284115a184 Add tests for CommentFormatter
I haven't really reviewed the outputs, but at least a) they don't
crash b) they will fail if the output suddenly changes (which could
cause problems due to caching).

Bug: T252555
Change-Id: I1bbcbc5dd17ce1e24b3622062f5e8df4baf5f389
2020-10-20 04:13:25 +02:00
Bartosz Dziewoński a29c49ae70 Better way to update expected test outputs
Use an environment variable "DISCUSSIONTOOLS_OVERWRITE_TESTS".

Change-Id: I017112b7d6b1df9497f01f3f97f34e0935ca16f8
2020-10-19 23:53:30 +02:00
jenkins-bot a2cf9cc978 Merge "Correctly generate timezone abbreviations for parsing" 2020-10-15 15:24:14 +00:00
Bartosz Dziewoński a1dc3a4896 Correctly generate timezone abbreviations for parsing
Also, add tests covering this and the previous bug fixes in this code
(T259818, T261706).

Note that the test data added in tests/cases/ doesn't exactly match
the entire configuration of the wiki, only the parts we want to cover.
This is unlike the data in tests/data/, which was literally copied
from the relevant wikis, and which is used as input for other tests.

Bug: T265500
Change-Id: I29a59a5952f6dc9fb5910434bb6bcc9dcdaa01a9
2020-10-15 12:11:25 +00:00
Bartosz Dziewoński c464d995c3 tests: Fix some typos
Change-Id: I99b14b8aae7416bd7a25f563fb07e35dc98a39e7
2020-10-14 22:14:59 +02:00
Bartosz Dziewoński 329df8c953 Parsing discussions converted to language variants
* Export parser data (date format, digits, timezone names, and
  messages for weekday/month names) converted to language variants
* Update the parsers to try matching using every variant, in case
  the page is displayed in non-default variant (and to avoid
  problems with incomplete variant conversion)

Bug: T259818
Change-Id: I04d73992cd31ce06fa79f87df0c0a53d7efc3c58
2020-09-16 22:07:07 +00:00
Bartosz Dziewoński 084f45128c Improve and document the files in tests/data/
* Remove 'wgMetaNamespace' and 'wgMetaNamespaceTalk', the same data
  exists in 'wgFormattedNamespaces'.

* Rename 'wgContentLang' to 'wgContentLanguage', to match its real
  name in JS config. MediaWiki doesn't use 'wgContentLang' anywhere,
  although the related PHP global is called $wgContLang.

* Document how I made these files, previously only mentioned in the
  commit message of e9c401e3aa.

Change-Id: I67f962812c155aedf41154e0d837e7feb5af972d
2020-09-01 01:50:33 +02:00
jenkins-bot 4be90d7494 Merge "Re-apply new reply API patches (again)" 2020-08-25 11:11:12 +00:00
jenkins-bot 5f8214a2e4 Merge "parser: Fix comment ranges when timestamp has entities" 2020-08-20 12:10:43 +00:00
Bartosz Dziewoński b706eac8bc Re-apply new reply API patches (again)
This reverts commit 4d7c98b97c.

Change-Id: I4100521efb687ec324d25e273a9c986fd5dac0d0
2020-08-19 20:05:42 +00:00
Bartosz Dziewoński 4d7c98b97c Revert new reply API (again)
Causes page corruption, in a new way we haven't seen before.

* Revert "Move page updating logic to controller.js"
  This reverts commit 54fdc6de06.
* Revert "ReplyWidget: Move clear methods from #teardown to #clear"
  This reverts commit 9b811a94e0.
* Revert "ApiDiscussionToolsEdit: Do not pass 'basetimestamp'"
  This reverts commit 7de5938a6f.
* Revert "Use DOMCompat::getOuterHTML instead of doc->saveHTML()"
  This reverts commit 7b2448d2f0.
* Revert "CommentController: Remove remains of client-side edit conflict handling"
  This reverts commit 2d038af705.
* Revert "Restore error message for when comment is deleted while replying"
  This reverts commit 655c0526d6.
* Revert "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
  This reverts commit 9d0fc184fe.
* Revert "Create a 'transcludedfrom' API endpoint"
  This reverts commit 5d8f3b9051.
* Revert "Edit API for replies"
  This reverts commit 8829a1a412.

Bug: T259855
Change-Id: I6419408c6194ec0afa6b8ee604b12c1a24c6ac7b
2020-08-13 20:19:29 +02:00
Bartosz Dziewoński 375bfe028e parser: Fix comment ranges when timestamp has entities
Previously, parser would output offsets that don't exist in their
containers, because we were pretending that entities are parts of
their neighboring text nodes.

Turns out it's much easier to do it right when going backwards.

Change-Id: I9bccca2d403f1a976ae517449989170cdd99721e
2020-08-11 20:41:06 +02:00
Bartosz Dziewoński f0225243e0 tests: Fix some issues with overwriting outputs from PHP tests
Follow-up to ccd9e411d2.

* Fix variable name in CommentTestCase::overwriteHtmlFile()
* Overwrite before assertions, because they abort execution if they fail

Change-Id: I5bba016ba93f9dd1994325ae82c3105ba11cf033
2020-08-11 06:45:45 +02:00
Ed Sanders 7b2448d2f0 Use DOMCompat::getOuterHTML instead of doc->saveHTML()
The latter results in lots of extra HTML entity encoding.

The former is built by the Parsing team and appears to result
in no unexpected changes elsewhere in the document.

As Parsoid's selser relies on HTML fragments being byte-for-byte
equal, these changes were resulting in wikitext normalisations
in untouched parts of the document ("dirty diffs").

Bug: T259855
Change-Id: Ib3cb605911e690ec3e8c2f9df25fd1a2e2849d7e
2020-08-07 21:31:38 +02:00
Bartosz Dziewoński ccd9e411d2 Allow updating the expected results when running PHP tests
This is similar to the code we already have in JS tests, but instead
of printing to the console where you have to copy-paste from, it just
overwrites the files.

Also, update all of the expected results by this method.

Changes in the expected outputs:
* In JSON files, the "warnings" are now always in the same place
  regardless of the type of the warning.
* In all HTML files, self-closing tags now include the trailing slash,
  some characters are no longer encoded as entities when not necessary,
  and attributes may be single-quoted when that makes them shorter.
* In Parsoid HTML files, the header is no longer terribly mangled.

Other notes:
* CommentParserTest.php: Change the output of serializeComments()
  to be in similar order as in JS, to reduce the diffs in this commit
  and because it's a better order for humans.
* modifier.test.js: Remove some hacks that were working around small
  inconsistencies between the previous expected outputs and the actual
  outputs.

Change-Id: I9f764640dae823321c0ac35898fa4db03f1ca364
2020-08-04 03:05:28 +02:00
jenkins-bot 889de1bcdf Merge "Improve detecting typed signatures" 2020-07-22 01:43:40 +00:00