Commit graph

96 commits

Author SHA1 Message Date
Ed Sanders 0b42aea276 CommentParser: Cache variables in getUsernameFromLink
Change-Id: I625e6ded3badd75a7a658c8d000576d0d165a18b
2022-02-04 19:35:18 +00:00
Ed Sanders 8ad1df7dc8 CommentParser: Name parts of return value from findSignature
Change-Id: I3a5ad36df0afdedc0aa9a15e5d83c5426b03b790
2022-02-04 19:34:18 +00:00
Bartosz Dziewoński f15693eefa Use class list everywhere for adding/checking CSS classes
In PHP, use DOMCompat::getClassList(), provided by Parsoid.
In JS, use `.classList`, available in all supported browsers.

This may fix some bugs where we were incorrectly checking for exactly
one class. The change in isOurGeneratedNode() is needed for
Ib2fa40c5fa389572b0e88ef558728fa06e3621b0.

Change-Id: Ia28d31678fd3d617b69280c4b7857755300fa515
2022-01-24 18:40:00 +01: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 34011b7a07 Parser: Pass in title of page being parsed
Will be used to parse selflinks in the future.

Change-Id: I2bc29d1c5c69cb6309f582f162f9af7d96ce8913
2022-01-12 21:17:59 +00:00
Bartosz Dziewoński ef7274d69e Move some helpers from CommentParser to CommentUtils
Change-Id: I0e323d3b75f47459a5548a13e9684f4c6ff4ba0c
2021-12-13 17:13:41 +01:00
Ed Sanders 8e4f08182e Add missing typehints
Change-Id: Ia25c5bea1834a3fdd26f32a9d5ed097789329824
2021-12-01 14:57:09 +00:00
Ed Sanders a86d308d66 CommentItem.php: Store timestamp object instead of string
We do something similar in CommentItem.js with a moment object.
The object can be converted to a string when required.

Change-Id: Id7221e9201db0d89c3b771574634c878c9515ca0
2021-11-09 16:37:45 +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 15747aba3a Merge "CommentParser: Remove outdated legacy ID algorithm" 2021-09-21 16:50:37 +00:00
Alexander Vorwerk 97a702cbc7 CommentParser: use IPUtils instead of the deprecated IP class
Bug: T291008
Change-Id: I0207940a642a32f2ca997b78387c9ff0af101599
2021-09-14 22:19:05 +02:00
Bartosz Dziewoński 9819df3288 CommentParser: Remove outdated legacy ID algorithm
Last changed in March (4a0802065c),
was only needed for about 2 weeks for compatibility with cached data.

Change-Id: I510238cb86a7b4d7ae5e8636716d1e9ca2d0e402
2021-09-07 17:41:30 +02:00
Bartosz Dziewoński a5099739a6 Improve notifications for comments posted in close succession
In case 4 and case 6, no notifications are expected. In all other
cases we now get the expected notifications.

Bug: T285528
Change-Id: I9e813bb3a053bc1232783f9eae1ad75672b4fa7e
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
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
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
jenkins-bot ef7073b8fd Merge "Simplify how warnings for IDs equal to legacy IDs are avoided" 2021-04-15 23:31:45 +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 b28290fa62 Simplify how warnings for IDs equal to legacy IDs are avoided
I don't like the extra parameter.

Follow-up to d05109b24d.

Change-Id: Ic0f403a816fd3182982002da326bb32d591ebcf7
2021-03-22 20:15:07 +00:00
Ed Sanders 4a0802065c Make IDs (to be used as URL hashes) wikitext safe
* Use hyphens instead of pipes a separators
* Use underscores for spaces in usernames

Change-Id: I6efd9739fc73e45002e50e64c43ce0de1c2f1239
2021-03-18 20:45:21 +01: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 f5059e6ea6 Don't detect comments within 'cite' elements too
Follow-up to 024a978ffd.

Bug: T275881
Change-Id: I53448ad22cd0531e7fd4aa0ea5d15782879cce14
2021-03-01 21:40:43 +01:00
jenkins-bot 0eb37a87df Merge "Don't detect comments within quotes" 2021-02-28 22:56:20 +00:00
Bartosz Dziewoński 024a978ffd Don't detect comments within quotes
Bug: T275881
Change-Id: I8f7a4279837bd95ebf5b604ff350c0a3f29c2c05
2021-02-28 22:49:48 +00:00
jenkins-bot 8bb5eea999 Merge "Improve signature detection to handle formatting on the timestamp" 2021-02-27 22:54:50 +00:00
jenkins-bot 49938a88dc Merge "Improve merging multiple comments on one paragraph" 2021-02-27 22:54:43 +00:00
Daimona Eaytoy 67096cb431 Stop using deprecated Language methods
Change-Id: I7cf21365df355a4a62f9e353be61aaa03ed58b9d
2021-02-27 14:48:49 +00: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 af082908a5 Improve merging multiple comments on one paragraph
The horrendous 11-line if() condition did not correctly handle
signatures wrapped in inline formatting markup, like <small>.

Instead, implement this logic in the code for skipping to the end
of a paragraph, which didn't exist yet when that condition was
added, but seems like a much better place to check this now.

Bug: T275934
Change-Id: I5cccff889b5e15b5f8fde0538bf4bccb22e762cf
2021-02-27 02:21:36 +01:00
Bartosz Dziewoński 35738b1f9b CommentParser: Replace getThreadStartTimestamp with getThreadStartComment
Change-Id: Ia8d878594306b5ce4039ca06d6dcec753e5dea28
2021-02-24 12:26:58 +00:00
Ed Sanders fa484e0c4a Don't allow CommentItem author to be null
Change-Id: Idb12bfa62e42bff521e872ab358b5ba9a8d24089
2021-02-22 20:55:35 +00:00
Bartosz Dziewoński 1998c983f1 computeId() can't return null
It used to return null for headings, but now it doesn't. Simplify some
code checking for that.

Change-Id: I28131c4aee89b901879b4c49953d6b15ed91b5e7
2021-02-13 00:08:15 +01:00
Ed Sanders d05109b24d Truncate user generated part of IDs to 80 characters
This ensures that IDs fit in a 255 character database field.

Bug: T273658
Change-Id: I3cfe4fce6a865b4343f0f01121cd696aa5f98b22
2021-02-03 15:04:58 +00:00
Bartosz Dziewoński c781b127c9 Handle category links at ends of comments affecting indentation
* Ignore rendering-transparent nodes between discussion comments.
* Improve isRenderingTransparentNode() so that <link> nodes
  representing TemplateStyles are not considered transparent,
  otherwise this would undo ae920b831f.
  Using a regexp from Parsoid.

Bug: T272746
Change-Id: I0b3c3251156ba6c4826abf5ba44ea93f80ebc01d
2021-01-26 04:55:03 +01: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
Bartosz Dziewoński 50ad5bb2b4 Ignore outdent templates at the beginning of comments
Bug: T264116
Change-Id: Iae9dbb30a1aead897cc274f655d3ecff4b297dbd
2020-12-14 21:35:56 +01:00
Bartosz Dziewoński ae920b831f Change which nodes are ignored at the beginning of comments again
While working on T270009, I noticed that <style> and <link> nodes
are treated differently, which seemed weird. Rewrite this again,
hopefully this is the last time.

The changed test cases also involve <area> and <input> nodes,
and the new results make more sense to me.

Bug: T264116
Change-Id: I3af90c84768a4b3dc53446927f4dba6f72175a2f
2020-12-14 21:33:50 +01:00
Bartosz Dziewoński 0fc71f60cd Skip to the end of the paragraph if it's just text, too
We've recently decided that we want to "extend" comments until
the end of the paragraph (e36dc8e78a,
d0ae6c4e44).

However, we still had this special case that did the opposite: it
ensured that if a comment ended in the middle of a text node, the
comment would not be extended to the end of the node. Remove it.

Note the change in the test file signatures-funny-formattedreply.html,
which actually covered this case specifically.

Change-Id: Id1384bb0c6e1a5f0c70f55efcb4caa240f230f07
2020-11-25 00:48:53 +01:00
Ed Sanders d0ae6c4e44 Skip end marker "forward" until a block tag is reached
The end marker is skipped forward until an open or close
block tag is reached. In tree traversal terms this means
moving either to the next sibling, or the parent (to skip
over close tags).

Bug: T256033
Change-Id: Iaa2c588698790d576ac4f9ecc126f58a082ef6b3
2020-11-23 15:08:29 +00:00
Ed Sanders 44a1bbcc59 Fix start node for comments following headings
The general rule is that comments start after their preceding
thread item, but when that is a heading we should skip past
the entire <h[1-6]> node to avoid making section edit links
part of the first comment.

Bug: T267988
Change-Id: Ia7f1b27e0a69a9aab7c7da743bf8549479304096
2020-11-19 23:48:30 +00:00
Thiemo Kreuz 8ffe0d55da Remove comments that literally repeat what the code says
Change-Id: Ib928cf61dc512fbbf39a3279789376d635a82c52
2020-11-11 09:31:59 +01:00
jenkins-bot e378a9122b Merge "Don't detect comments within headings" 2020-11-05 16:56:02 +00:00
Bartosz Dziewoński bed717d329 Move getHeadlineNodeAndOffset() to utils
Needed by I7d35098d672d0edb50d49e22de1686d5cc83b60e.

Change-Id: I44bf927213de570fe9de43e485e09cfae6778eef
2020-11-05 16:11:30 +01:00
Bartosz Dziewoński 986e83ee61 Fix getHeadlineNodeAndOffset() returning text nodes
The condition was wrong, it could return either an element child with
.mw-headline, or a non-element child.

Bug: T267284
Change-Id: I28cda22ee8c5fe4a3259621adddd647b31291703
2020-11-05 16:09:35 +01:00
Bartosz Dziewoński 1626242863 Don't detect comments within headings
Bug: T267068
Change-Id: Id134f15e086fd070801c4b1d836dbfbf9bf444ad
2020-11-04 21:57:33 +01:00
libraryupgrader fb6706a606 build: Updating mediawiki/mediawiki-codesniffer to 32.0.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPrivate
* MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationProtected
* MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPublic

Additional changes:
* Dropped .inc files from .phpcs.xml (T200956).

Change-Id: I340d6b573e9ae2a99085fb19a705fcf567b03f92
2020-10-29 10:53:01 +00:00
Ed Sanders 3aca622894 Treat headings like comments now they have IDs
Use the same logic for marking ranges in the document, and ensure
that the heading range does not include section edit links or
section numberings.

Change-Id: I782caafc34fee2a822b0a17b24dd6b9528202eca
2020-10-28 12:38:18 +00:00
Bartosz Dziewoński ba8434e2e0 Add legacy IDs as of wmf/1.36.0-wmf.14
Bug: T264478
Change-Id: I099e1068fedc25d671cd1245ac8b32941dca7232
2020-10-22 22:52:59 +02:00
Bartosz Dziewoński 0ddc171c8a Add oldest timestamp in the thread to heading IDs
To avoid old threads re-appearing on popular pages when someone
uses a vague title (e.g. dozens of threads titled "question" on
[[Wikipedia:Help desk]]: https://w.wiki/fbN), include the oldest
timestamp in the thread (i.e. date the thread was started) in the
heading ID.

Bug: T264478
Change-Id: If918bfd5e025248923d1939bc86916697ead95a0
2020-10-22 02:19:21 +02:00
Bartosz Dziewoński b09bbfe668 Disambiguate comments by parent ID, rather than sequential numbers
Sequential numbers aren't great because they change when an earlier
comment is archived. Parent comment/heading IDs should change less
often.

This also makes much more sense for disambiguating subsections,
e.g. a dozen identical ===Votes=== sections for a dozen proposals.

Bug: T264478
Change-Id: I466454984fd919ebef35f2b37ddb5d86dc842996
2020-10-22 02:19:21 +02:00