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
Also, in ThreadItem::getSinglePageTransclusionTitle(), we don't need
this terribly complicated method.
Change-Id: If02c09aaa2f4dd66b2bc253a1edec4ea107564ee
We were rendering the preview in a completely different way from how
we would add the real reply, and the results would be different
sometimes, particularly for multi-line comments with messed-up markup.
Render it server-side instead, in a very similar way to real replies
(generating a DOM list node and transforming it through Parsoid),
although without the whole context of the page to improve performance.
We can remove a lot of client-side code that was used solely for this.
This will allow the preview to accurately display the signatures when
we change how they are added (T278442), without us having to implement
those changes again from scratch for the preview.
Change-Id: I53341f4d4075c25b67ec3b3032bff9b8a880dcd3
Goal:
-----
Finishing the work from Iadb7757debe000025e52770ca51ebcf24ca8ee66
by changing CommentParser::parse() to return a data object, instead of
the whole parser.
Changes:
--------
ThreadItemSet.php:
ThreadItemSet.js:
* New data class to access the results of parsing a discussion. Most
methods and properties are moved from CommentParser with no changes.
CommentParser.php:
Parser.js:
* parse() returns a new ThreadItemSet.
* Remove methods moved to ThreadItemSet.
* Placeholder headings are generated slightly differently, as we process
things in a different order.
* Grouping threads and computing IDs/names is no longer lazy. We always
needed IDs/names anyway.
* computeId() explicitly uses a ThreadItemSet to check the existing IDs
when de-duplicating.
controller.js:
* Move the code for turning some nodes annotated by CommentFormatter
into a ThreadItemSet (previously a Parser) from controller#init to
ThreadItemSet.static.newFromAnnotatedNodes, and rewrite it to handle
assigning parents/replies and recalculating legacy IDs more nicely.
* mw.dt.pageThreads is now a ThreadItemSet.
Change-Id: I49bfe019aa460651447fd383f73eafa9d7180a92
Goal:
-----
To have a method like CommentParser::parse(), which just takes a node
to parse and a title and returns plain data, so that we don't need to
keep track of the config to construct a CommentParser object (the
required config like content language is provided by services) and
we don't need to keep that object around after parsing.
Changes:
--------
CommentParser.php:
* …is now a service. Constructor only takes services as arguments.
The node and title are passed to a new parse() method.
* parse() should return plain data, but I split this part to a separate
patch for ease of review: I49bfe019aa460651447fd383f73eafa9d7180a92.
* CommentParser still cheats and accesses global state in a few places,
e.g. calling Title::makeTitleSafe or CommentUtils::getTitleFromUrl,
so we can't turn its tests into true unit tests. This work is left
for future commits.
LanguageData.php:
* …is now a service, instead of a static class.
Parser.js:
* …is not a real service, but it's changed to behave in a similar way.
Constructor takes only the required config as argument,
and node and title are instead passed to a new parse() method.
CommentParserTest.php:
parser.test.js:
* Can be simplified, now that we don't need a useless node and title
to test internal methods that don't use them.
testUtils.js:
* Can be simplified, now that we don't need to override internal
ResourceLoader stuff just to change the parser config.
Change-Id: Iadb7757debe000025e52770ca51ebcf24ca8ee66
The Data class contained utilities for two unrelated purposes.
Split each half to a separate class.
Notably, this improves the signature of the getLocalData() function.
Change-Id: Icde615fb9d483fee1f352c34909b37f8ffde8081
(suggested by PhpStorm)
composer.json:
* Document required PHP extensions
Parser.js:
* Remove incorrect param documentation
* Fix some typos in comments (missing parentheses)
CommentParser.php:
* Fix some typos in comments (missing parentheses)
ImmutableRange.php:
* Remove unused property
* Add a `throw` to indicate that code path is unreachable
SubscribedNewCommentPresentationModel.php:
* Add missing `return false`
CommentParserTest.php:
* Remove unnecessary pass-by-reference
CommentModifierTest.php:
* Remove unused variable
CommentParserTest.php:
* Don't construct Element objects directly. PHP's DOMElement allows
it, but Parsoid/Dodo's doesn't, and we use the latter for static
analysis. This generates all kinds of confusing warnings.
Change-Id: Ia9598ebea0e99830dd485296e94a9d96acc4b258
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
